Closed Bug 524781 Opened 16 years ago Closed 3 years ago

xpcshell-tests: turn on strict & werror by default

Categories

(Testing :: General, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED INVALID
mozilla1.9.3a1

People

(Reporter: lusian, Unassigned)

References

Details

(Keywords: helpwanted)

Attachments

(5 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091027 Minefield/3.7a1pre (.NET CLR 3.5.30729) Build Identifier: Run the xpcshell test with the Javascript options 'strict' & 'werror' on by default. Right now the test runs with the 'strict' on, and some of the existing tests fail when the 'werror' option is on. Reproducible: Always Steps to Reproduce: 1. Run 'make xpcshell-tests' with the '-S' option Actual Results: Failed Expected Results: Passed
Depends on: 520743, 521143, 521151, 521154, 521191
URL: 521189
URL: 521189
Depends on: 521189
Two tests fail on JS strict mode (I submitted patches for everything else for now): TEST-UNEXPECTED-FAIL | c:/mozilla-build/mozilla-central-devel/obj-i686-pc-mingw32/_tests/xpcshell/test_extensionmanager/unit/test_bug470377_3.js | true == false JS frame :: c:/mozilla-build/mozilla-central-devel/obj-i686-pc-mingw32/_tests/xpcshell/test_extensionmanager/unit/test_bug470377_3.js :: run_test :: line 94 TEST-UNEXPECTED-FAIL | c:/mozilla-build/mozilla-central-devel/obj-i686-pc-mingw32/_tests/xpcshell/test_extensionmanager/unit/test_bug470377_4.js | false == true JS frame :: c:/mozilla-build/mozilla-central-devel/obj-i686-pc-mingw32/_tests/xpcshell/test_extensionmanager/unit/test_bug470377_4.js :: run_test :: line 96
Attachment #409242 - Flags: review?(doug.turner) → review+
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: checkin-needed
Attachment #409240 - Flags: review?(benjamin) → review+
Attachment #409600 - Flags: review? → review?(benjamin)
Attachment #409600 - Flags: review?(benjamin) → review+
Attached patch json2.js (obsolete) — Splinter Review
Attachment #410739 - Flags: review?
Comment on attachment 409240 [details] [diff] [review] patch for tests that fails on js strict mode [Checkin: Comment 6] http://hg.mozilla.org/mozilla-central/rev/4346a668d379
Attachment #409240 - Attachment description: patch for tests that fails on js strict mode → patch for tests that fails on js strict mode [Checkin: Comment 6]
Comment on attachment 409242 [details] [diff] [review] xpcom/tests/unit/test_localfile.js [Checkin: Comment 7 & 16 & 20] http://hg.mozilla.org/mozilla-central/rev/a27c6a3bcd6d
Attachment #409242 - Attachment description: xpcom/tests/unit/test_localfile.js → xpcom/tests/unit/test_localfile.js [Checkin: Comment 7]
Attachment #409600 - Attachment description: Fix errors in JS strict mode → Fix errors in JS strict mode [Checkin: Comment 8]
Assignee: nobody → lusian
Severity: trivial → minor
Status: NEW → ASSIGNED
Flags: in-testsuite+
OS: Windows XP → All
Hardware: x86 → All
Whiteboard: [needs approval for c-n: m-1.9.2 and probably m-1.9.1]
Target Milestone: --- → mozilla1.9.3a1
Version: unspecified → Trunk
Comment on attachment 409240 [details] [diff] [review] patch for tests that fails on js strict mode [Checkin: Comment 6] "approval1.9.2=?": for nsExtensionManager.js.in. "No" risk, 'var' addition.
Attachment #409240 - Flags: review?(beltzner)
Comment on attachment 409600 [details] [diff] [review] Fix errors in JS strict mode [Checkin: Comment 8] "approval1.9.2=?": "No" risk, 'var' addition.
Attachment #409600 - Flags: review?(beltzner)
Jae-Seong, please, in future, file separate bugs for non-test files.
Um, this doesn't need to be fixed for 1.9.2. Or 1.9.1. Or anywhere but mozilla-central. There's no urgent need for thinko-catching behavior, it's just nice (perhaps very nice, sometimes) to have, and we can live a little longer if we have to without it on branches. Jae-Seong, did you mean to request review from a particular person on the json2.js bit?
Jeff, I don't know whom to ask to review json2.js. I was just hoping somebody picks up and reviews the file. I think I fixed all the existing tests. json2.js & bug 520743 are pending.
Attachment #410739 - Flags: review? → review?(sayrer)
Comment on attachment 410739 [details] [diff] [review] json2.js have the default case throw
Attachment #410739 - Flags: review?(sayrer) → review-
Attachment #413054 - Flags: review?(jwalden+bmo)
Comment on attachment 413054 [details] [diff] [review] xpcshell-tests: turn on strict & werror by default [don't check-in yet] Jae-Seong, did you check that the (3) comm-central apps too are ready for this?
Attachment #409242 - Attachment description: xpcom/tests/unit/test_localfile.js [Checkin: Comment 7] → xpcom/tests/unit/test_localfile.js [Checkin: Comment 7 & 16]
(In reply to comment #15) > (From update of attachment 413054 [details] [diff] [review]) > > Jae-Seong, did you check that the (3) comm-central apps too are ready for this? Ooops, I didn't. Thank you for saying that. I will look at comm-central, too.
Attachment #410739 - Attachment is obsolete: true
Attachment #413554 - Flags: review?(sayrer)
Depends on: 530028
Depends on: 530032
Depends on: 530034
Depends on: 530035
(In reply to comment #17) > I will look at comm-central, too. Thanks for filing blocking bugs :-) Note that tests are considered part of the application(s) components: the Testing component is mainly for the harness itself...
Attachment #409242 - Attachment description: xpcom/tests/unit/test_localfile.js [Checkin: Comment 7 & 16] → xpcom/tests/unit/test_localfile.js [Checkin: Comment 7 & 16 & 20]
Depends on: 530292
Depends on: 530376
Blocks: 532022
No longer blocks: 532022
Depends on: 532022
Depends on: 532035
Depends on: 532037
Bug 514559 forbids the use of octal literals on the strict mode, and I can't think of a strict-legal expression as simple as the old one. e.g., dir.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0744); Can you suggest any?
I was thinking maybe (7 << 6) | (4 << 3) | 4 at one point, but maybe something like this is better: function octal() { var r = 0; for (var i = 0; i < arguments.length; i++) r = (r << 3) + arguments[i]; return r; } dir.createUnique(Ci.nsIFile.DIRECTORY_TYPE, octal(7, 4, 4));
Attachment #413054 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 413054 [details] [diff] [review] xpcshell-tests: turn on strict & werror by default [don't check-in yet] There are still many things to do; calendar does not build on m-c. Octal literals are deprecated.
Attachment #413054 - Attachment description: xpcshell-tests: turn on strict & werror by default → xpcshell-tests: turn on strict & werror by default [don't check-in yet]
Comment on attachment 409600 [details] [diff] [review] Fix errors in JS strict mode [Checkin: Comment 8] (Giving up on branches.)
Attachment #409600 - Flags: review?(beltzner)
Comment on attachment 409240 [details] [diff] [review] patch for tests that fails on js strict mode [Checkin: Comment 6] (Giving up on branches.)
Attachment #409240 - Flags: review?(beltzner)
Whiteboard: [needs approval for c-n: m-1.9.2 and probably m-1.9.1]
Depends on: 551231
Depends on: 551521
Attachment #413554 - Flags: review?(sayrer) → review?(jwalden+bmo)
Comment on attachment 413554 [details] [diff] [review] default thorws Error Review of attachment 413554 [details] [diff] [review]: ----------------------------------------------------------------- This file got removed when I moved JSON testing into js/src/tests, and the file itself didn't get transferred. So I don't think this patch is needed any more.
Attachment #413554 - Flags: review?(jwalden+bmo)

The bug assignee didn't login in Bugzilla in the last 7 months.
:ahal, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: lusian → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(ahal)
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(ahal)
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: