Closed Bug 1050855 Opened 6 years ago Closed 6 years ago

Remove --log-file mochitest option, replaced by the structured logs arguments

Categories

(Testing :: Mochitest, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: akachkach, Assigned: akachkach)

References

Details

Attachments

(1 file, 4 obsolete files)

When bug 1039833 lands, we'll be able to output logs produced by mochitest in multiple formats (--log-raw, --log-tbpl, --log-mach, etc.) and so there's no need for --log-file anymore (that currently dumps what the JS test harness produces: JSON surrounded by our 'unicode delimiters').
Assignee: nobody → akachkach
Status: NEW → ASSIGNED
See Also: → 1027665
So, this was used by the android runtestsremote.py after all!
This patch should keep it working while removing the command line option (as this should only be used by the test harness, not devs).

New try run: https://tbpl.mozilla.org/?tree=Try&rev=5b81005f56a4
Attachment #8471089 - Attachment is obsolete: true
Attachment #8471089 - Flags: review?(ahalberstadt)
Attachment #8471229 - Flags: review?(ahalberstadt)
Comment on attachment 8471229 [details] [diff] [review]
0001-Bug-1050855-Remove-log-file-and-file-level-options-r.patch

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

Looks good, but I'd like to try to preserve the old behaviour if possible (by using the new command line options)

::: testing/testsuite-targets.mk
@@ +29,5 @@
>  
>  RUN_MOCHITEST_B2G_DESKTOP = \
>    rm -f ./$@.log && \
>    $(PYTHON) _tests/testing/mochitest/runtestsb2g.py --autorun --close-when-done \
> +    --console-level=INFO \

Could you still save $@.log using --log-tbpl or --log-mach or something? Otherwise I'm sure there will be some developer who still uses these old targets complaining :)
Attachment #8471229 - Flags: review?(ahalberstadt) → review-
Indeed! This should do it. (it'll format the logs with the TBPL-like formatting that we used to have before structured logs)

I also had to add a small bugfix in runtests.py to keep using options.logFile (since it's used by the android harness to pass a param at URL building and get the JS logs in a file instead of stdout).
Attachment #8471229 - Attachment is obsolete: true
Attachment #8471872 - Flags: review?(ahalberstadt)
Comment on attachment 8471872 [details] [diff] [review]
0001-Bug-1050855-Remove-log-file-and-file-level-options-r.patch

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

Thanks, looks good! If this was tested on try, then looks good to land. You may want to make a note to dev.platform saying these options are changing slightly (let me know if you'd prefer I do it).
Attachment #8471872 - Flags: review?(ahalberstadt) → review+
carry r+ forward;

Had to manually add a default value for fileLevel (since it's no longer provided via command line).

New try run: https://tbpl.mozilla.org/?tree=Try&rev=ce8e5e1d3632
Attachment #8471872 - Attachment is obsolete: true
Attachment #8473261 - Flags: review+
Carry r+ forward;

(replaced None by "" for logFile, since it's supposed to be a string)

New try run: https://tbpl.mozilla.org/?tree=Try&rev=273c50364a42
Attachment #8473261 - Attachment is obsolete: true
Attachment #8473964 - Flags: review+
Try run looks OK!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ad6931ea53e4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.