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

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Ahmed Kachkach [:akachkach]]

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').

Comment 1

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1050855-Remove-log-file-and-file-level-options-r.patch

Try run: https://tbpl.mozilla.org/?tree=Try&rev=41ddd3b322e6

Comment 2

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1050855-Remove-log-file-and-file-level-options-r.patch

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

Comment 3

[Andrew Halberstadt [:ahal]]

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 :)

Comment 4

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1050855-Remove-log-file-and-file-level-options-r.patch

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).

Comment 5

[Andrew Halberstadt [:ahal]]

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).

Comment 6

[Ahmed Kachkach [:akachkach]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=dfb94fdc1efc

Comment 7

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1050855-Remove-log-file-and-file-level-options-r.patch

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

Comment 8

[Ahmed Kachkach [:akachkach]]

Attachment: 0001-Bug-1050855-Remove-log-file-and-file-level-options-r.patch

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

Comment 9

[Ahmed Kachkach [:akachkach]]

Try run looks OK!

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad6931ea53e4

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ad6931ea53e4