Closed Bug 1122909 Opened 9 years ago Closed 9 years ago

Show clearer error message in jstests.py when shell could not be found at the given path

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38

People

(Reporter: ProgramFOX, Assigned: ProgramFOX)

Details

Attachments

(1 file, 4 obsolete files)

When you pass a shell path to jstests.py that's not valid, it gives an error message that does not clearly tell you that the entered path is invalid. This has confused me some times, so I created this patch, which shows a clear error message when the shell could not be found.
Attachment #8550710 - Flags: review?(terrence)
Comment on attachment 8550710 [details] [diff] [review]
Patch - show clear error message in jstests.py when shell could not be found

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

Nice!
Attachment #8550710 - Flags: review?(terrence) → review+
Added r=terrence to the commit message, and pushed to try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e095744a3228
Attachment #8550710 - Attachment is obsolete: true
Attachment #8550851 - Flags: review+
The try push busted. Seems like there are cases where it's valid if options.js_shell is None, so this gave a TypeError when passing None to isfile. I added a None check.
Attachment #8550851 - Attachment is obsolete: true
Attachment #8550861 - Flags: review?(terrence)
Comment on attachment 8550861 [details] [diff] [review]
Patch - show clear error message in jstests.py when shell could not be found

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

Right, I should have remembered that we don't pass a shell for manifest expansion. Good find.
Attachment #8550861 - Flags: review?(terrence) → review+
Attached patch jstests-shellnotfound4.diff (obsolete) — Splinter Review
Added r=terrence to the patch. Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b72894f7070c
Attachment #8550861 - Attachment is obsolete: true
Attachment #8551933 - Flags: review+
The Mac static analysis on Try couldn't run because I pushed my patch on top of a stale m-c clone [0]; I pulled, qrefreshed, exported and uploaded the patch again (code changes weren't necessary). New try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8aa7436331cc


  [0]: http://logs.glob.uno/?c=mozilla%23developers#c1144303
Attachment #8551933 - Attachment is obsolete: true
Attachment #8552513 - Flags: review+
The busted build in the Try server worked after restarting it, and the only failed test on Win7 is caused by bug 1123404. I believe this can be checked in, adding checkin-needed.
Keywords: checkin-needed
Assignee: nobody → programfox
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/a913718a2f80
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: