Closed Bug 1417051 Opened 7 years ago Closed 7 years ago

Report full test filenames in logs to allow queries in active data

Categories

(Remote Protocol :: Marionette, enhancement)

Product:
Remote Protocol
Component:
Marionette
Version:
Version 3
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox59 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox59 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

Details

Attachments

(4 files)

Bug 1417051 - Don't report test start/end twice.
59 bytes, text/x-review-board-request
impossibus
: review+
Details
Bug 1417051 - Report relative path to Marionette test modules starting from cwd.
59 bytes, text/x-review-board-request
impossibus
: review+
Details
Bug 1417051 - Mozharness has to run Marionette tests from tests folder.
59 bytes, text/x-review-board-request
impossibus
: review+
Details
Bug 1417051 - Reorganize Firefox UI tests in test package for full path names in log files.
59 bytes, text/x-review-board-request
impossibus
: review+
Details
At the moment we only report the file name of the test file. This doesn't allow us to easily query test results via active data. Similar to other test harnesses Marionette has to add the full path to the test file to the report. Here the report for mochitest canvas tests: http://fxtestr.us-west-2.elasticbeanstalk.com/unittest?branch=autoland&path=dom%2Fcanvas%2Ftest&since=today-1week For Marionette we only cover the 13 xpcshell tests because they report the full test file name: http://fxtestr.us-west-2.elasticbeanstalk.com/unittest?branch=autoland&path=testing%2Fmarionette&since=today-1week I kinda would like to see those reports also for Marionette.
The output looks already pretty fine, but mozharness needs to be adjusted to run the tests from the correct subfolder, so the paths match up with the folder structure in mozilla-central: https://treeherder.mozilla.org/logviewer.html#?job_id=148833169&repo=try&lineNumber=5475 09:22:23 INFO - TEST-PASS | tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_click.py TestLegacyClick.test_clicking_an_element_that_is_not_displayed_raises | took 359ms to 09:22:23 INFO - TEST-PASS | testing/marionette/harness/marionette_harness/tests/unit/test_click.py TestLegacyClick.test_clicking_an_element_that_is_not_displayed_raises | took 359ms
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
I got the Marionette working with the correct relative path, but the Firefox UI tests are a bit trickier. Here the current output: https://treeherder.mozilla.org/logviewer.html#?job_id=149045772&repo=try&lineNumber=6189 [task 2017-12-01T08:21:41.574Z] 08:21:41 INFO - TEST-START | tests/firefox-ui/tests/puppeteer/test_software_update.py TestSoftwareUpdate.test_os_version As you can see those do not package correctly, and as such we loose the path from where they are located in the tree. It means I have to do the following: 1) Adjust the packaging of the tests and the harness, so that they all end-up in a folder structure like that: > tests/firefox-ui/harness/ > tests/firefox-ui/tests/testing/firefox-ui/tests/puppeteer/test_software_update.py 2) Adjust the mozharness script for firefox ui tests, to report the relative path with `tests/firefox-ui/tests` as root.
Finally I also got the Firefox UI tests working with the new paths: https://treeherder.mozilla.org/logviewer.html#?job_id=149180357&repo=try&lineNumber=5162 > 13:58:34 INFO - TEST-PASS | testing/firefox-ui/tests/puppeteer/test_software_update.py TestSoftwareUpdate.test_build_info | took 3443ms
Attachment #8933361 - Flags: review?(mjzffr)
Marionette tests on Android are failing now because runtest.py cannot be found: INFO - /builds/worker/workspace/build/venv/bin/python: can't open file '/builds/worker/workspace/build/tests/marionette/tests/harness/marionette_harness/runtests.py': [Errno 2] No such file or directory I will check this Monday morning.
After checking the Android failures I noticed that there is nothing special going on in any of the mozharness files. As such I checked the harness files in more details, and noticed that harnesses like Mochitest and Reftest have a helper in place which removes a possible `tests` prefix from the logged test path: https://dxr.mozilla.org/mozilla-central/rev/574f4f58fe09dd590ea892406e237318c31705b4/testing/mochitest/runtests.py#185-193 To be consistent we should implement the same logic for Marionette.
Blocks: 1313591
With the most recent push the patches are ready for review now. A try job can be found here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af02dbc60aa5f7c25484854d89446686363fccca&selectedJob=149529097&bugfiler (The mnh job got fixed by checking that the path exists before using os.path.relpath on it)
Comment on attachment 8933337 [details] Bug 1417051 - Don't report test start/end twice. https://reviewboard.mozilla.org/r/204256/#review210642
Attachment #8933337 - Flags: review?(mjzffr) → review+
Comment on attachment 8933338 [details] Bug 1417051 - Report relative path to Marionette test modules starting from cwd. https://reviewboard.mozilla.org/r/204258/#review210646 ::: testing/marionette/harness/marionette_harness/runner/base.py:809 (Diff revision 7) > filename = os.path.basename(filename) > return self.filename_pattern.match(filename) > > def _log_skipped_tests(self): > for test in self.manifest_skipped_tests: > - name = os.path.basename(test['path']) > + self.logger.test_start(test['path']) So for skipped tests you're logging the full test-archive path, not the relative path. Is this okay for ActiveData, or do we not care about skipped tests?
Attachment #8933338 - Flags: review?(mjzffr) → review+
Comment on attachment 8933361 [details] Bug 1417051 - Mozharness has to run Marionette tests from tests folder. https://reviewboard.mozilla.org/r/204272/#review210648 ::: testing/mozharness/scripts/marionette.py:340 (Diff revision 7) > self.mkdir_p(env['MOZ_UPLOAD_DIR']) > env = self.query_env(partial_env=env) > > + try: > + cwd = self._query_tests_dir() > + except: Let's at least catch `Exception` here and log a message about the exception, in case it's something unusual.
Attachment #8933361 - Flags: review?(mjzffr) → review+
Comment on attachment 8933793 [details] Bug 1417051 - Reorganize Firefox UI tests in test package for full path names in log files. https://reviewboard.mozilla.org/r/204724/#review210650
Attachment #8933793 - Flags: review?(mjzffr) → review+
Comment on attachment 8933338 [details] Bug 1417051 - Report relative path to Marionette test modules starting from cwd. https://reviewboard.mozilla.org/r/204258/#review210646 > So for skipped tests you're logging the full test-archive path, not the relative path. Is this okay for ActiveData, or do we not care about skipped tests? Good catch. I indeed missed to make it relative. Will fix it.
Comment on attachment 8933338 [details] Bug 1417051 - Report relative path to Marionette test modules starting from cwd. https://reviewboard.mozilla.org/r/204258/#review210646 > Good catch. I indeed missed to make it relative. Will fix it. I didn't notice that because it is only logged when a complete test module gets skipped. I only tested with the @skip decorator. I dislike that we have this logging in base.py and not testcases.py. So I had to duplicate the `_fix_test_path()` helper. If it warrants the efforts, we might be able to kill it when moving to pytest?
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ba56ac3d6603 Don't report test start/end twice. r=maja_zf https://hg.mozilla.org/integration/autoland/rev/e7662cc12ccb Report relative path to Marionette test modules starting from cwd. r=maja_zf https://hg.mozilla.org/integration/autoland/rev/eadb73ce9eb7 Mozharness has to run Marionette tests from tests folder. r=maja_zf https://hg.mozilla.org/integration/autoland/rev/33e35ba1c14b Reorganize Firefox UI tests in test package for full path names in log files. r=maja_zf
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: