779094 - Add a restartless extension in Blocklist

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Infrastructure, defect)

Description

[Remus Pop (:RemusPop)]

We need a new blocklist for a restartless extension in https://mozqa.com/data/firefox/addons/blocklist/softblock_extension/
Please see bug 760139 comment 17.

Comment 1

[Remus Pop (:RemusPop)]

Attachment: patch v1 (litmus-data)

This would also need to be copied in the mozmill-tests repository.

Comment 2

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

Comment on attachment 647529 [details] [diff] [review]
patch v1 (litmus-data)

Is there a reason we need a separate blocklist? I was expecting that the restartless addon would be added to the existing blocklist. I'll hold off on reviewing for now.

Comment 3

[Maniac Vlad Florin (:vladmaniac)]

Remus, Andreea, can we sort this out soonish? This bug is blocking bug 760139 which is also blocking bug 684679 which is a test - I am beginning to think we have lots of refactoring or updating to do each time we consider developing new stuff :(

Comment 4

[Remus Pop (:RemusPop)]

No important reason, just to keep things clear.

Comment 5

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v2 (litmus-data)

Added the restartless-inlinesettings extension to the existing blocklist.
It also needs to be added in data/addons/extensions.

Comment 6

[Maniac Vlad Florin (:vladmaniac)]

Comment on attachment 647933 [details] [diff] [review]
patch v2 (litmus-data)

So can you please add this in data/addons/extensions as well? 
We really want our patches to be complete. 

Hint: In the future, when not certain of something or you need help, 
you can submit a WIP (Work in Progress) patch with a feedback? request. 
Just keep in mind that we only submit this kind of patches for feedback? only, 
and not for review 

So I will consider your current patch as a WIP and if so, its a f+

Comment 7

[Andreea Matei [:AndreeaMatei]]

From what Remus said, this patch needed to change the blocklist from mozqa.com first, then it will in mozmill-tests (where we need to add the extension because in mozqa already exists).
Please let me know if that is correct or should I upload a patch with blocklist and add extension in mozmill-tests from the start.

Comment 8

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

From what I understand, attachment 647933 [details] [diff] [review] is for litmus-data, which already has the necessary extension. On this basis please request review again and I will land it.

Then follow-up with a second patch for mozmill-tests including the change to the blocklist and the extension.

Comment 9

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

Comment on attachment 647933 [details] [diff] [review]
patch v2 (litmus-data)

This patch fails to apply with the following message:

unable to find 'data/addons/blocklist/softblock_extension/blocklist.xml' for patching

I believe this should be firefox/addons/blocklist/softblock_extension/blocklist.xml

Comment 10

[Maniac Vlad Florin (:vladmaniac)]

Yes indeed, this patch is for litmus-data then firefox/addons/blocklist/softblock_extension/blocklist.xml 

Its really bad I missed this, but still I was just giving only feedback to this patch in terms of style and approach.

Comment 11

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v3 (litmus-data) [checked in]

Sorry about that, modified for litmus-data.

Comment 12

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

Comment on attachment 648311 [details] [diff] [review]
patch v3 (litmus-data) [checked in]

Thanks. Landed as:
https://hg.mozilla.org/qa/litmus-data/rev/32769bdd3ffa

Comment 13

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

Let's use this bug for the mozmill-tests patch with the blocklist change and extension.

Comment 14

[Andreea Matei [:AndreeaMatei]]

Attachment: patch v1 (all branches)

Patch for mozmill-tests with extension restartless_inlinesettings added and blocklist modified.

Comment 15

[Maniac Vlad Florin (:vladmaniac)]

Comment on attachment 648317 [details] [diff] [review]
patch v1 (all branches)

This actually looks good and works as expected. Patch applies cleanly at least for default and aurora branch. 

On the above considerations, r+ and moving over to Dave

Comment 16

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

Comment on attachment 648317 [details] [diff] [review]
patch v1 (all branches)

Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/ae2554e59eab (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/2fa560db1795 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/f8e87d39bedc (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/f328f7a440ee (release)

Comment 17

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

(In reply to Dave Hunt (:davehunt) from comment #13)
> Let's use this bug for the mozmill-tests patch with the blocklist change and
> extension.

In the future we should try to separate infrastructure (test cases) and mozmill tasks and not combine both in the same bug. That will reduce the amount of confusion for everyone. There will always be an open bug for mozmill a change like this can be incorporated.

Comment 18

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

(In reply to Henrik Skupin (:whimboo) from comment #17)
> (In reply to Dave Hunt (:davehunt) from comment #13)
> > Let's use this bug for the mozmill-tests patch with the blocklist change and
> > extension.
> 
> In the future we should try to separate infrastructure (test cases) and
> mozmill tasks and not combine both in the same bug. That will reduce the
> amount of confusion for everyone. There will always be an open bug for
> mozmill a change like this can be incorporated.

I'm not sure I follow the wording of your comment Henrik, but if you mean we should keep litmus-data and mozmill-tests bugs separate in future then I'm fine with that.

Comment 19

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

(In reply to Dave Hunt (:davehunt) from comment #18)
> I'm not sure I follow the wording of your comment Henrik, but if you mean we
> should keep litmus-data and mozmill-tests bugs separate in future then I'm
> fine with that.

litmus-data is just a subgroup of all infrastructure tasks. But yes, that's right.