Closed Bug 937412 Opened 6 years ago Closed 6 years ago

Syntax errors in test files don't report the actual syntax error

Categories

(Testing :: XPCShell Harness, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla28

People

(Reporter: markh, Assigned: markh)

Details

Attachments

(1 file, 1 obsolete file)

STR:
* Introduce a SyntaxError in an xpcshell test that otherwise passes.
* Run the modified test

Actual:
* Error log has:

0:02.73 TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | Source file o:/src/mozilla-git/central/obj-release/_tests/xpcshell/services/sync/tests/unit/test_errorhandler.js contains SyntaxError
JS frame :: o:\src\mozilla-git\central\testing\xpcshell\head.js :: loadTailFile :: line 420
JS frame :: _load_files@o:\src\mozilla-git\central\testing\xpcshell\head.js:436
JS frame :: o:\src\mozilla-git\central\testing\xpcshell\head.js :: _execute_test :: line 344
JS frame :: @-e:1

Note there is no information about where or what the syntax error actually is.

Expected:
* Error log has something like:

 0:02.73 TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | Source file o:/src/mozilla-git/central/obj-release/_tests/xpcshell/services/sync/tests/unit/test_errorhandler.js contains SyntaxError
Diagnostic: SyntaxError: missing ) after argument list at o:/src/mozilla-git/central/obj-release/_tests/xpcshell/services/sync/tests/unit/test_errorhandler.js:187
JS frame :: o:\src\mozilla-git\central\testing\xpcshell\head.js :: loadTailFile :: line 420
JS frame :: _load_files@o:\src\mozilla-git\central\testing\xpcshell\head.js:436
JS frame :: o:\src\mozilla-git\central\testing\xpcshell\head.js :: _execute_test :: line 344
JS frame :: @-e:1

Note the new line starting with "Diagnostic: ".  This is the output with the attached patch applied.
Attachment #830533 - Flags: feedback?(ted)
Comment on attachment 830533 [details] [diff] [review]
Print "diagnostic" element if it exists.

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

Is this worth adding a selftest for?
http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/selftest.py
Attachment #830533 - Flags: feedback?(ted) → feedback+
Attached patch With selftestSplinter Review
Added a selftest.
Assignee: nobody → mhammond
Attachment #830533 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #832530 - Flags: review?(ted)
Comment on attachment 832530 [details] [diff] [review]
With selftest

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

Thanks. Sorry  for adding an extra review lag, you could have landed the patch and done the test as a follow-up if you wanted.
Attachment #832530 - Flags: review?(ted) → review+
Comment on attachment 832530 [details] [diff] [review]
With selftest

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

markh: This patch is malformed. Produced patches should contain "a/" and "b/" prefixes in the filenames. I'm not sure what command you used to produce the patch (it looks like git something), but whatever it is, it's not the "right" way. I don't use Git for m-c patches, but I've heard good things about https://github.com/mozilla/moz-git-tools. I assume it "just works."
(In reply to Gregory Szorc [:gps] from comment #4)
> markh: This patch is malformed. Produced patches should contain "a/" and
> "b/" prefixes in the filenames.

heh - I thought that was a feature ;)  I'll adjust my workflow.  FTR, https://etherpad.mozilla.org/moz-git-tools
https://hg.mozilla.org/mozilla-central/rev/4bfd3281f531
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.