833745 - Mozmill test for installing and uninstalling a soft-blocklisted extension

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Tests, defect, P2)

Description

[Andrei Eftimie]

A followup to bug 684679 which only tests Hard-Blocked extesnions.
This test should be very similar, but test a Soft-Blocked extension.

Comment 1

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.0.patch

As in bug 684679, tests will install an addon and will:
-set extensions.blockedlist.url to a local file in which addon is set as blocked 
-open Blocklist window, check if addon is softblocked 
-uncheck the disabled checkbox so addon will still be enabled
-uninstall the addon. 

Reports: 
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d376d9ff
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d376c45c
http://mozmill-crowd.blargon7.com/#/functional/report/b3aefd29a5c41bda1cc43616d37fe783

Comment 2

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 782999 [details] [diff] [review]
patch_v1.0.patch

Review of attachment 782999 [details] [diff] [review]:
-----------------------------------------------------------------

::: data/addons/blocklist/softblock_extension/blocklist.xml
@@ +6,1 @@
>        <targetApplication id="{ec8030f7-c20a-464f-9b0e-13a3a9e97384}">

We don't need this change since we're using another addon which has the right properties to be softblocked.

::: lib/ui/addons_blocklist.js
@@ +104,5 @@
>          nodeCollector.queryNodes(".softBlockedAddon");
>          break;
> +      case "disableCheckbox":
> +        nodeCollector.queryAnonymousNode("class", "disableCheckbox");
> +        break

This has to be alphabetically sorted, please move it up. Also a semicolon is missing after break.

::: tests/functional/restartTests/testAddons_installUninstallSoftBlocklistedExtension/test1.js
@@ +15,5 @@
> +
> +const ADDON = {
> +    name: "Inline Settings (Restartless)",
> +    id: "[e-mail]",
> +    url: BASE_URL + "addons/extensions/restartless_inlinesettings.xpi"

We use 2 spaces for indentation.

::: tests/functional/restartTests/testAddons_installUninstallSoftBlocklistedExtension/test2.js
@@ +55,5 @@
> +
> +  var disableCheckbox = blocklistWindow.getElement({type: "disableCheckbox",
> +                                                    parent: softBlockedAddon});
> +  controller.click(disableCheckbox);
> +

No need for this blank line, we have a block where we click and wait for a behavior.

@@ +58,5 @@
> +  controller.click(disableCheckbox);
> +
> +  assert.waitFor(function () {
> +    return !softBlockedAddon.getNode().checked;
> +  }, "The addon is still enabled");

"The addon has been enabled" expresses better the action here, cause we are doing that by not accepting the default disabled action of the blocklist dialog.
More than this, I think we have handled this wrong in the hardblocked test as well. We force a restart between the tests, when we have this dialog opened and with a restart button as well. Maybe this is the reason for this test failing in 2.0.
I'll raise a bug to fix this in both tests.

Comment 3

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch v1.1.patch

Comment 4

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 783725 [details] [diff] [review]
patch v1.1.patch

Review of attachment 783725 [details] [diff] [review]:
-----------------------------------------------------------------

We'll be ready to land with this fixed.

::: tests/functional/restartTests/testAddons_installUninstallSoftBlocklistedExtension/test4.js
@@ +5,5 @@
> +"use strict";
> +
> +// Include required modules
> +var { assert } = require("../../../../lib/assertions");
> +var addons = require("../../../../lib/addons");

One more nit, sorry I didn't see it before. Please move addons above assert.

Comment 5

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.2.patch

Comment 6

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.2.patch

Comment 7

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 784391 [details] [diff] [review]
patch_v1.2.patch

Review of attachment 784391 [details] [diff] [review]:
-----------------------------------------------------------------

Great, it's landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/d15baaa18780 (default)

Thanks! Please check backporting.

Comment 8

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.2.patch (esr17)

Reports :
Ubuntu
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8bf6491
Mac
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8c17de0
Windows
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8beb357

Comment 9

[Cosmin Malutan, [:cosmin-malutan]]

Attachment: patch_v1.2.patch (Beta & Release)

Reports:
Windows
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8c39a5b
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8c28751
Ubuntu
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8c35b39
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8c2dd84
Mac
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8c33a72
http://mozmill-crowd.blargon7.com/#/functional/report/bccc2ea9ac1e478fa074c699d8c68ecf

Comment 10

[Andreea Matei [:AndreeaMatei]]

Comment on attachment 786907 [details] [diff] [review]
patch_v1.2.patch (Beta & Release)

Review of attachment 786907 [details] [diff] [review]:
-----------------------------------------------------------------

Pushed to:
http://hg.mozilla.org/qa/mozmill-tests/rev/6bdc078944b3 (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/c08f18e8eebf (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/ae2ebf439316 (esr17)

Thanks Cosmin!

Comment 11

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Just to note, we don't usually backport new tests, but thanks for getting this one complete!

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Dave Hunt (:davehunt) from comment #11)
> Just to note, we don't usually backport new tests, but thanks for getting
> this one complete!

That's totally correct. Reason is that we really don't want to mess up with beta and release testing. So please keep that in mind in the future. Thanks!