David Burns :automatedtester Reporter
	Description • 7 years ago

There are tests that do not get run if there is an error in the python file (like when imports are not properly). This is not getting picked up by wptrunner as an error/failure and it should.

In bug 1416407 the import was not working but none of the harnesses picked it up as a failure.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 1 • 7 years ago

I will fix this.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 2 • 7 years ago

Example of erroneous output:

> % ./mach wpt testing/web-platform/tests/webdriver/tests/set_window_rect.py
>  0:03.34 LOG: MainThread INFO Using 1 client processes
>  0:03.40 SUITE_START: MainThread 1
>  0:03.40 LOG: MainThread INFO Running reftest tests
>  0:03.40 LOG: MainThread INFO Starting http server on 127.0.0.1:8000
>  0:03.41 LOG: MainThread INFO No reftest tests to run
>  0:03.41 LOG: MainThread INFO Running wdspec tests
>  0:03.42 LOG: Thread-TestrunnerManager-1 INFO Starting runner
>  0:03.42 LOG: MainThread INFO Starting http server on 127.0.0.1:8001
>  0:03.43 LOG: MainThread INFO Starting http server on 127.0.0.1:8443
>  0:03.45 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:31259) Full command: /home/ato/src/gecko/obj-x86_64-pc-linux-gnu/dist/bin/geckodriver --marionette-port 2828 --host 127.0.0.1 --port 4445
> (pid:31259) "1510676583517	geckodriver	INFO	geckodriver 0.19.1 (b0a6b4678b2f7dfb499328946b95366775f71edd 2017-11-14)"
>  0:03.47 PROCESS_OUTPUT: Thread-TestrunnerManager-1 (pid:31259) "1510676583540	geckodriver	INFO	Listening on 127.0.0.1:4445"
>  0:03.95 LOG: Thread-TestrunnerManager-1 INFO WebDriver HTTP server listening at http://127.0.0.1:4445/
>  0:03.95 TEST_START: Thread-TestrunnerManager-1 /webdriver/tests/set_window_rect.py
>  0:04.25 LOG: Thread-Log INFO STDOUT: ============================= test session starts ==============================
>  0:04.25 LOG: Thread-Log INFO STDOUT: platform linux2 -- Python 2.7.13, pytest-2.9.1, py-1.4.31, pluggy-0.3.1 -- /usr/bin/python2.7
>  0:04.25 LOG: Thread-Log INFO STDOUT: rootdir: /home/ato/src/gecko/testing/web-platform/tests/webdriver/tests, inifile: 
>  0:04.25 LOG: Thread-Log INFO STDOUT: plugins: hypothesis-3.6.1
>  0:04.25 LOG: Thread-Log INFO STDOUT: collecting ... 
>  0:04.30 LOG: Thread-Log INFO STDOUT: collected 0 items / 1 errors
>  0:04.30 LOG: Thread-Log INFO STDOUT: ==================================== ERRORS ====================================
>  0:04.30 LOG: Thread-Log INFO STDOUT: _____________________ ERROR collecting set_window_rect.py ______________________
>  0:04.30 LOG: Thread-Log INFO STDOUT: testing/web-platform/tests/webdriver/tests/set_window_rect.py:4: in <module>
>  0:04.30 LOG: Thread-Log INFO STDOUT:     import asdjasd
>  0:04.30 LOG: Thread-Log INFO STDOUT: build/mach_bootstrap.py:364: in __call__
>  0:04.30 LOG: Thread-Log INFO STDOUT:     module = self._original_import(name, globals, locals, fromlist, level)
>  0:04.30 LOG: Thread-Log INFO STDOUT: E   ImportError: No module named asdjasd
>  0:04.30 LOG: Thread-Log INFO STDOUT: =========================== 1 error in 0.05 seconds ============================
>  0:04.30 TEST_END: Thread-TestrunnerManager-1 OK
>  0:04.30 LOG: Thread-TestrunnerManager-1 INFO Pausing until the browser exits
>  0:04.31 LOG: Thread-TestrunnerManager-1 INFO No more tests
>  0:04.37 LOG: Thread-TestrunnerManager-1 WARNING u'runner_teardown' ()
>  0:04.37 LOG: MainThread INFO Running testharness tests
>  0:04.37 LOG: MainThread INFO No testharness tests to run
>  0:04.37 LOG: MainThread INFO Got 0 unexpected results
>  0:04.38 SUITE_END: MainThread 
> Summary
> =======
> 
> Ran 1 tests
> Expected results: 1
> Unexpected results: 0
> 
> OK
>  0:04.46 LOG: MainThread INFO Closing logging queue
>  0:04.46 LOG: MainThread INFO queue closed

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 3 • 7 years ago

This code looks suspicious:

>     def do_wdspec(self, session_config, path, timeout):
>         harness_result = ("OK", None)
>         subtest_results = pytestrunner.run(path,
>                                            self.server_config,
>                                            session_config,
>                                            timeout=timeout)
>         return (harness_result, subtest_results)

	James Graham [:jgraham]
	Comment 5 • 7 years ago
mozreview-review

Comment on attachment 8928228 [details]
Bug 1416410 - Record wdspec harness outcomes.

https://reviewboard.mozilla.org/r/199450/#review204550

Assuming you tested this.

	James Graham [:jgraham]
	Comment 8 • 7 years ago
mozreview-review

Comment on attachment 8928246 [details]
Bug 1416410 - Fix syntax error in editable wdspec test.

https://reviewboard.mozilla.org/r/199472/#review204568

/me cries

	Dave Hunt [:davehunt] [he/him] ⌚BST
	Comment 9 • 7 years ago
mozreview-review

Comment on attachment 8928228 [details]
Bug 1416410 - Record wdspec harness outcomes.

https://reviewboard.mozilla.org/r/199448/#review206320

This was discussed on IRC. I believe ato is instead going to use a try/except around the pytest invocation, which should catch such collection errors as well as other exceptions that may be raised.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 10 • 7 years ago

(In reply to Dave Hunt (:davehunt) from comment #9)

> This was discussed on IRC. I believe ato is instead going to use a
> try/except around the pytest invocation, which should catch such
> collection errors as well as other exceptions that may be raised.

Indeed.

I think we will want to keep the HarnessResultRecorder that taps
into pytest_collectreport as it can also report skipped tests.  I
will add the try…except block for added safety, however.

	Pulsebot
	Comment 12 • 7 years ago

Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d3ce6118908
Record wdspec harness outcomes. r=jgraham

	Tiberius Oros[:tiberius_oros]
	Comment 13 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6d3ce6118908

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 14 • 7 years ago

There are tests marked as successful on Treeherder now which include failures in the log. Did this landing regress something?

https://treeherder.mozilla.org/logviewer.html#?job_id=147393593&repo=autoland&lineNumber=4281

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 15 • 7 years ago

(In reply to Henrik Skupin (:whimboo) from comment #14)

> There are tests marked as successful on Treeherder now which
> include failures in the log. Did this landing regress something?
> 
> https://treeherder.mozilla.org/logviewer.html#?job_id=147393593&repo=autoland&lineNumber=4281

Since https://bugzil.la/1411045 we include a bit more information
when displaying WebDriverExceptions and responses.  Is it this extra
information that you are thinking of?

Looking at a few randomly chosen child tests that are failing, they
are all expected to fail.  Which ones are you suspecting this patch
regressed?

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 16 • 7 years ago

Sorry, I may have been wrong. I didn't check the meta data for those tests, and if those are marked as expected fail.