1216104 - Remove --html-output from the flags of gaiatest

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::UI Tests, defect)

Description

[Johan Lorenzo [:jlorenzo]]

Steps to reproduce
1. Run a gaiatest command with --html-output=index.html in a folder that is writable
2. Wait for the command to complete
3. Try to open index.html

Results
There is no index.html file. Running --log-html=index.html with the exact same flags, in the same folder works.

I'm not sure if that's a regression or not.

Comment 1

[Martijn Wargers (dead)]

Regression from bug 1196489 perhaps?

Comment 2

[Johan Lorenzo [:jlorenzo]]

I just tried after a revert of bug 1196489 and the output is back again. I'll find the cause of the issue.

Comment 3

[Johan Lorenzo [:jlorenzo]]

I found out that HTMLReportingArguments is defined in marionette-client itself[1]. Our usage of it is straight forward[2]. I don't see anywhere in Gecko where that class is called.

Moreover, --log-html give the same data (+ more) than --html-output used to give. I wonder if we shouldn't just remove --html-output from gaiatest[2]. 

Henrik, do you know if there are some other places (than gaiatest) where HTMLReportingArguments is used? If not, is there a reason why we keep it, compared --log-html?

[1] https://dxr.mozilla.org/mozilla-central/rev/9605da94e75d61598d3c00f01a12d1b6bc427a6c/testing/marionette/client/marionette/runner/mixins/reporting.py#226
[2] https://github.com/mozilla-b2g/gaia/blob/master/tests/python/gaia-ui-tests/gaiatest/runtests.py#L34
[3] https://dxr.mozilla.org/mozilla-central/search?q=HTMLReportingArguments&redirect=true&case=true

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Functionality-wise nothing should have changed by moving over to argparse. But maybe something was missed to change? Andrew mentored this work so I will forward the ni? to him.

Comment 5

[Andrew Halberstadt [:ahal]]

I don't think it's used anywhere other than gaiatest.. I believe it's safe to remove, though I'd check with AutomatedTester first.

--log-html is impemented as part of mozlog, and is probably a different format than what --html-output was. But yes, I think ideally we'd remove --html-output and use --log-html instead. You'd probably need to use a custom formatter to get the exact same output as before though. It's a matter of whether it's worth it or not.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] JohanLorenzo:bug-1216104 > mozilla-b2g:master

Comment 7

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8677977 [details] [review]
[gaia] JohanLorenzo:bug-1216104 > mozilla-b2g:master

Based on previous comments, we can remove the --html-output flag.

Testing done around this patch:
* `gaiatest --help`, the option is no more present => OK
* `gaiatest --html-output=index.html REGULAR GAIA OPTIONS`, an error is returned => OK:
> gaiatest: error: unrecognized arguments: --html-output=index.html
* `gaiatest --log-html=index.html REGULAR GAIA OPTIONS`, the suite is executed and th report is still generated and returns the same data as before => OK

NI'ing William, so he's aware the option will disappear.

Comment 8

[William Hsu [:whsu]]

Thanks for your actively help.
You're the man!

I have removed incorrect flag (html-output) and set the correspondent flag (log-html) on Jenkins.
Have a nice weekend!
:)

Comment 9

[Martijn Wargers (dead)]

Comment on attachment 8677977 [details] [review]
[gaia] JohanLorenzo:bug-1216104 > mozilla-b2g:master

Fwiw, I don't feel really qualified to review this code.

Comment 10

[Paul Yang [:pyang] (away)]

Comment on attachment 8677977 [details] [review]
[gaia] JohanLorenzo:bug-1216104 > mozilla-b2g:master

I'm ok for removing this option but need to take care with existing coupled scripts in jenkins

Comment 11

[Johan Lorenzo [:jlorenzo]]

Discussed with Paul on IRC, William has made the changes in TPE Jenkins. In the MTV one, we've been using log-html. 

Landed in master at: https://github.com/mozilla-b2g/gaia/commit/6b883a6e8b44f498593dbe9521654634ce612d59

Now that Gaia doesn't use --html-output anymore, it sounds okay to remove it from marionette_client. NI'ing :AutomatedTester to chime in, based on comment 5.

Comment 12

[Dave Hunt [:davehunt] [he/him] ⌚BST]

This was similar to bug 1125098 but for the HTML report. I can't find a dupe so I may have not raised one for removing the HTML report option. See also bug 1065410.