Closed Bug 1123945 Opened 5 years ago Closed 5 years ago

mach mochitest-chrome --no-autorun won't allow running

Categories

(Testing :: Mochitest, defect)

x86
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla38

People

(Reporter: jryans, Assigned: jryans)

Details

Attachments

(1 file)

If I use the command

mach mochitest-chrome browser/devtools/webide --no-autorun

then clicking the button to actually run fails with:

JavaScript error: chrome://mochikit/content/tests/SimpleTest/setup.js, line 253: TypeError: RunSet is undefined
Comment on attachment 8552058 [details] [diff] [review]
0002-Bug-1123945-Fix-no-autorun-harness-option.-r-jmaher.patch

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

I like most of this, but I do want to learn more before r+.

::: testing/mochitest/tests/SimpleTest/setup.js
@@ +58,5 @@
>  }
>  
>  if (config.testRoot == "chrome" || config.testRoot == "a11y") {
> +  for (let p in params) {
> +    if (params[p] == [1]) {

I don't understand the [1] and [0], is there a specific javascript feature for this?
Attachment #8552058 - Flags: review?(jmaher) → review-
Comment on attachment 8552058 [details] [diff] [review]
0002-Bug-1123945-Fix-no-autorun-harness-option.-r-jmaher.patch

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

::: testing/mochitest/tests/SimpleTest/setup.js
@@ +58,5 @@
>  }
>  
>  if (config.testRoot == "chrome" || config.testRoot == "a11y") {
> +  for (let p in params) {
> +    if (params[p] == [1]) {

No, nothing that magical.

|parseQueryString| is called with useArrays: true, so it puts each query param's value in an array, I think to allow for multiple values with the same param name?

I'll confess I don't understand what's expected to happen here at all, so I was mostly just flailing to get this to work. :)

I looked through version history, but did not see any recent changes to explain why this broke...  so I tried to make what seemed like the smallest change possible.
See comment 3.  Maybe a different approach is needed?  Not sure.
Flags: needinfo?(jmaher)
I see where you get it.  have you tested this patch with mochitest-plain?  on try server to see what android and b2g do?

Assuming this works for everything, I would like a small comment explaining why we are comparing to an array.  This wouldn't make sense just reading it a year from now, without some context in this bug.
Flags: needinfo?(jmaher)
Seems some contexts don't load the file with "let" support.  For such a small change, I've just gone back to "var".

Let's see what happens.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=09555dbc59aa
Comment on attachment 8552058 [details] [diff] [review]
0002-Bug-1123945-Fix-no-autorun-harness-option.-r-jmaher.patch

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

please add a comment to the code to indicate why you have the []
Attachment #8552058 - Flags: review- → review+
(In reply to Joel Maher (:jmaher) from comment #8)
> Comment on attachment 8552058 [details] [diff] [review]
> 0002-Bug-1123945-Fix-no-autorun-harness-option.-r-jmaher.patch
> 
> Review of attachment 8552058 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> please add a comment to the code to indicate why you have the []

Thanks! I've got a comment in the one[1] on Try now.  Was going to re-request review once I was sure it looked good, but this works too! :)

Looking okay, except for some kind of Mac build failure, waiting on a rebased Try run to confirm.

[1]: https://hg.mozilla.org/try/rev/9bda75f63db7
With the additional run[1], it looks like the Mac issues were fixed after rebase.

[1]: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d796ff6e72ff
https://hg.mozilla.org/mozilla-central/rev/8b6b57a1d6eb
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla38
Component: Mochitest Chrome → Mochitest
You need to log in before you can comment on or make changes to this bug.