Closed Bug 915824 Opened 9 years ago Closed 8 years ago
move more CSP tests from content/base/test into csp subdir
Most of the CSP tests were moved in bug 903080, but a few got missed 'cause they're named stealthily. test_bug548193.html file_bug548193.sjs file_bug558431.html file_bug650386_content.sjs test_bug650386_redirect_301.html test_bug650386_redirect_302.html test_bug650386_redirect_303.html test_bug650386_redirect_307.html file_bug650386_report.sjs file_bug702439.html file_bug717511_2.html file_bug717511.html
We should rename these tests to be descriptive while we're at it - just have to make sure the bug # is in a comment, preferably at the top of the test_*.html file.
I will flag one but there are five parts (each move is a single patch), but they depend on the order (which isn't nice but we can change that if we want to...)
Assignee: nobody → yeukhon
Status: NEW → ASSIGNED
Brief try build (in-progress): https://tbpl.mozilla.org/?tree=Try&rev=fc20f9457222 Local builds look good (tested for each patch)
A quick comment: redirect tests have been "re-implemented" in csp/ directory with a quick glance. Therefore I added xcsp prefix to the file names. Injection is assosicated with XCSP at the time so it was named with xcsp prefix. The other two without xcsp.. I am just waiting to see if we should add xcsp or not.. In general, I think *it might be a good idea* to name XCSP-only tests with xcsp so that when we remove XCSP support from Firefox (I assume this is what we will do!) we can easily remove xcsp... Sorry for the spam. Thanks!
did you use hg rename/move/mv or did you just delete and addremove the files?
I did hg move (addremove got me in trouble, we can talk about that one on IRC). Or I think I did hg move for most of them. Do they not just show up as R and A in the end? I also have to fix comments and file path after I did hg move.
Look at your raw diffs... they're different for the different techniques. regarding xcsp: if the test makes ZERO sense for the spec compliant header, that's fine, use "xcsp" in the file name. If we can convert it to work with CSP 1.0 or 1.1, then don't because we'll end up tweaking the tests to use the new parser/syntax/etc.
Comment on attachment 8368955 [details] [diff] [review] part_1_xcsp_report_in_json_bug548193.patch Review of attachment 8368955 [details] [diff] [review]: ----------------------------------------------------------------- As we discussed in IRC last Friday, please collapse the moves into one patch and use hg mv to relocate the files. (Please also be sure you've got a recent version of mercurial as briansmith suggested older hg versions can mess up history of mv'ed files.
As discussed, I am using Mercurial Distributed SCM (version 2.8.2). Try looks good: https://tbpl.mozilla.org/?tree=Try&rev=4412ef6c0137
Comment on attachment 8370587 [details] [diff] [review] move_csp_test_v2.patch Review of attachment 8370587 [details] [diff] [review]: ----------------------------------------------------------------- r=me. Please make sure the bug #s are preserved in the test files you renamed to not have bug numbers. ::: content/base/test/csp/test_301_redirect.html @@ +70,5 @@ > > SimpleTest.waitForExplicitFinish(); > > // save this for last so that our listeners are registered. > +document.getElementById('content_iframe').src = 'file_redirect_content.sjs?301'; Please maintain the bug number in this file somewhere (in a comment at the top would probably be fine). Actually, please make sure the bug number is referenced somewhere in each file where you change the name from a bug number to its functionality (this way it's easier to search by bug number). ::: content/base/test/csp/test_csp_report.html @@ +1,5 @@ > <!DOCTYPE HTML> > <html> > +<!-- > +https://bugzilla.mozilla.org/show_bug.cgi?id=548193 > +--> Yeah, like this. This is good.
Attachment #8370587 - Flags: review?(sstamm) → review+
Carrying over r+ from Sid. Added bug numbers to all sjs and html files except ^headers^. Try looks good: https://tbpl.mozilla.org/?tree=Try&rev=3ef73fce48a5
This effectively re-enabled a lot of tests that were disabled on B2G because their platform-specific manifests weren't updated. Backed out. https://hg.mozilla.org/integration/mozilla-inbound/rev/2e5a57e18585 https://tbpl.mozilla.org/php/getParsedLog.php?id=34626241&tree=Mozilla-Inbound
Modified b2g.json, b2g-desktop.json and b2g-debug.json to reflect the name changes. Try looks good, all green: https://tbpl.mozilla.org/?tree=Try&rev=c0d449acd813
Attachment #8377019 - Flags: review+
Attachment #8375192 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.