Closed Bug 1086619 Opened 5 years ago Closed 5 years ago

Combine all mixed content blocker mochitests in one directory

Categories

(Core :: DOM: Security, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: ckerschb, Assigned: ckerschb)

Details

Attachments

(2 files, 2 obsolete files)

It would be great if we have all mixed content blocker tests in one directive so we can run them all at once without the need of selecting each single one individually. We have the same thing for CSP tests. I think it would make testing way easier, especially since we are working on 'loadinfo' patches where we need to run CSP and Mixed content blocker tests all the time.
s/directive/directory

Christoph has been working on too many CSP bugs ;)
Summary: Combine all mixed content blocker mochitests in one directive → Combine all mixed content blocker mochitests in one directory
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Attached patch bug_1086619_move_mcb_tests.patch (obsolete) — Splinter Review
Tanvi, you wrote all (or at least) most of the mixedcontent tests, can you make sure that I haven't forgotten to move one?

Also, do we need an additional reviewer for that? If so, who would that be?
Attachment #8511388 - Flags: review?(tanvi)
Review-note: Bug 946065 bitrotted the patches in this bug, I will un-bitrot and upload a new one soon.
Here we go - unbitrotted the patch!
Attachment #8511388 - Attachment is obsolete: true
Attachment #8511388 - Flags: review?(tanvi)
Attachment #8512272 - Flags: review?(tanvi)
Using 'hg mv' instead of 'mv' which should make reviewing way easier :-)
Attachment #8512272 - Attachment is obsolete: true
Attachment #8512272 - Flags: review?(tanvi)
Attachment #8512864 - Flags: review?(tanvi)
Attachment #8512864 - Flags: review?(jst)
Attachment #8512864 - Flags: review?(jst) → review+
Comment on attachment 8512864 [details] [diff] [review]
bug_1086619_move_all_mcb_tests_into_one_directory.patch

Other tests to consider adding here:

browser tests:
browser/base/content/test/general/browser_bug822367.js
browser/base/content/test/general/browser_bug906190.js
browser/base/content/test/general/browser_bug902156.js
browser/base/content/test/general/browser_mixedcontent_securityflags.js
browser/base/content/test/general/browser_bug1045809.js

Webconsole related:
browser/devtools/webconsole/test/browser_webconsole_block_mixedcontent_securityerrors.js
browser/devtools/webconsole/test/browser_webconsole_bug_737873_mixedcontent.js 
browser/devtools/webconsole/test/test-bug-737873-mixedcontent.html
browser/devtools/webconsole/test/browser_webconsole_allow_mixedcontent_securityerrors.js
browser/devtools/webconsole/test/test-mixedcontent-securityerrors.html
Attachment #8512864 - Flags: review?(tanvi) → review+
(In reply to Tanvi Vyas [:tanvi] from comment #6)
> Comment on attachment 8512864 [details] [diff] [review]
> bug_1086619_move_all_mcb_tests_into_one_directory.patch
> 
> Other tests to consider adding here:
> 
> browser tests:
> browser/base/content/test/general/browser_bug822367.js
> browser/base/content/test/general/browser_bug906190.js
> browser/base/content/test/general/browser_bug902156.js
> browser/base/content/test/general/browser_mixedcontent_securityflags.js
> browser/base/content/test/general/browser_bug1045809.js
> 
> Webconsole related:
> browser/devtools/webconsole/test/
> browser_webconsole_block_mixedcontent_securityerrors.js
> browser/devtools/webconsole/test/browser_webconsole_bug_737873_mixedcontent.
> js 
> browser/devtools/webconsole/test/test-bug-737873-mixedcontent.html
> browser/devtools/webconsole/test/
> browser_webconsole_allow_mixedcontent_securityerrors.js
> browser/devtools/webconsole/test/test-mixedcontent-securityerrors.html

Thanks Tanvi, I agree, we should bundle those together as well.
https://hg.mozilla.org/mozilla-central/rev/4eab8ea8276e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
(In reply to Tanvi Vyas [:tanvi] from comment #6)
> Comment on attachment 8512864 [details] [diff] [review]
> bug_1086619_move_all_mcb_tests_into_one_directory.patch
> 
> Other tests to consider adding here:
> 
> browser tests:
> browser/base/content/test/general/browser_bug822367.js
> browser/base/content/test/general/browser_bug906190.js
> browser/base/content/test/general/browser_bug902156.js
> browser/base/content/test/general/browser_mixedcontent_securityflags.js
> browser/base/content/test/general/browser_bug1045809.js
> 
All of these are tagged with mcb in http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/general/browser.ini#262

> Webconsole related:
> browser/devtools/webconsole/test/
> browser_webconsole_block_mixedcontent_securityerrors.js
tagged

> browser/devtools/webconsole/test/browser_webconsole_bug_737873_mixedcontent.
> js 
Not tagged: http://mxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/test/browser.ini#277

> browser/devtools/webconsole/test/
> browser_webconsole_allow_mixedcontent_securityerrors.js
tagged

We need to tag browser_webconsole_bug_737873_mixedcontent.js and try moving all of these tests to dom/security as well.  I'll find out if that is possible.
Flags: needinfo?(tanvi)
Thinking more about this and chatting on #fx-team, I've convinced myself that moving the devtools and browser tests out of /devtools and /browser doesn't make sense.  The UI code for MCB lives in /browser, so although the tests rely on platform behaving a certain way, we shouldn't move those tests.  Similarly, devtools code lives in /devtools, so we shouldn't move the tests that test mcb error messages either.

The only item that remains is tagging one test, which I will do:
> browser/devtools/webconsole/test/browser_webconsole_bug_737873_mixedcontent.
> js 
Not tagged: http://mxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/test/browser.ini#277
Just adding a tag to a webconsole test.
Flags: needinfo?(tanvi)
Attachment #8677582 - Flags: review?(past)
Attachment #8677582 - Flags: review?(past) → review+
You need to log in before you can comment on or make changes to this bug.