Open Bug 975560 Opened 6 years ago Updated 3 years ago

Missing xpchell.ini manifest files should be considered to be a test failure.

Categories

(Testing :: XPCShell Harness, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

2.0 S4 (20june)

People

(Reporter: dhylands, Unassigned)

References

Details

(Whiteboard: [fxos:media])

I'm investigating bug 959327 and I decided to take a look at a recent TBPL log and see what tests were actually being run, and I discovered this:

06:18:45     INFO - Calling ['/builds/slave/test/build/venv/bin/python', 'runtestsb2g.py', '--adbpath=/builds/slave/test/build/emulator/b2g-distro/out/host/linux-x86/bin/adb', '--b2gpath=/builds/slave/test/build/emulator/b2g-distro', '--emulator=arm', '--logcat-dir=/builds/slave/test/build', '--manifest=tests/xpcshell_b2g.ini', '--use-device-libs', '--testing-modules-dir=/builds/slave/test/build/tests/modules', '--symbols-path=http://pvtbuilds.pvt.build.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-central-generic/20140221040929/b2g-30.0a1.en-US.android-arm.crashreporter-symbols.zip', '--busybox=/builds/slave/test/build/busybox'] with output_timeout 1000
06:18:46     INFO -  /builds/slave/test/build/venv/local/lib/python2.7/site-packages/mozrunner/utils.py:19: UserWarning: Module mozinfo was already imported from /builds/slave/test/build/tests/xpcshell/mozinfo.py, but /builds/slave/test/build/venv/lib/python2.7/site-packages is being added to sys.path
06:18:46     INFO -    import pkg_resources
06:20:36     INFO -  Cleaning files from previous run..
06:21:41     INFO -  Included file '/builds/slave/test/build/tests/xpcshell/tests/toolkit/devtools/debugger/tests/unit/xpcshell.ini' does not exist
06:21:41     INFO -  Included file '/builds/slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/tests/unit_aus_update/xpcshell.ini' does not exist
06:21:41     INFO -  Included file '/builds/slave/test/build/tests/xpcshell/tests/toolkit/mozapps/update/tests/unit_base_updater/xpcshell.ini' does not exist
06:21:41     INFO -  pushing /system/bin/busybox
06:21:41     INFO -  waiting for system-message-listener-ready...
06:21:41     INFO -  done

I think that the missing manifest files should be considered errors.

What seems to have happened, is that these tests used to be run, but a bunch of restructuring was done, and the xpcshell.ini files are not being packaged, so the tests are now silently not run.

I'm going to propose that these missing files be considered an error.

I talked to ahal on IRC, and he said he thinks that all we need to do is change:

    mp = manifestparser.TestManifest(strict=False)

to be:

    mp = manifestparser.TestManifest(strict=True)

in http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/runxpcshelltests.py#789

But first, I need to figure out why there are missing xpcshell.ini files in the first place.
Assignee: nobody → dhylands
Blocks: 959327
That seems to work. I did a try run with just that change and it showed up as an orange X (although it should probably show up as a red X - failed):

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

Log extracted from: https://tbpl.mozilla.org/php/getParsedLog.php?id=35074241&tree=Try&full=1
14:51:39     INFO - Calling ['/builds/slave/test/build/venv/bin/python', 'runtestsb2g.py', '--adbpath=/builds/slave/test/build/emulator/b2g-distro/out/host/linux-x86/bin/adb', '--b2gpath=/builds/slave/test/build/emulator/b2g-distro', '--emulator=arm', '--logcat-dir=/builds/slave/test/build', '--manifest=tests/xpcshell_b2g.ini', '--use-device-libs', '--testing-modules-dir=/builds/slave/test/build/tests/modules', '--symbols-path=http://pvtbuilds.pvt.build.mozilla.org/pub/mozilla.org/b2g/try-builds/dhylands@mozilla.com-cfd74e5e5c1b/try-generic/b2g-30.0a1.en-US.android-arm.crashreporter-symbols.zip', '--busybox=/builds/slave/test/build/busybox'] with output_timeout 1000
14:51:40     INFO -  /builds/slave/test/build/venv/local/lib/python2.7/site-packages/mozrunner/utils.py:19: UserWarning: Module mozinfo was already imported from /builds/slave/test/build/tests/xpcshell/mozinfo.py, but /builds/slave/test/build/venv/lib/python2.7/site-packages is being added to sys.path
14:51:40     INFO -    import pkg_resources
14:53:31     INFO -  Cleaning files from previous run..
14:54:37     INFO -  Traceback (most recent call last):
14:54:37     INFO -    File "runtestsb2g.py", line 205, in main
14:54:37     INFO -      **options.__dict__):
14:54:37     INFO -    File "/builds/slave/test/build/tests/xpcshell/runxpcshelltests.py", line 1336, in runTests
14:54:37     INFO -      self.buildTestList()
14:54:37     INFO -    File "/builds/slave/test/build/tests/xpcshell/remotexpcshelltests.py", line 455, in buildTestList
14:54:37     INFO -      xpcshell.XPCShellTests.buildTestList(self)
14:54:37     INFO -    File "/builds/slave/test/build/tests/xpcshell/runxpcshelltests.py", line 795, in buildTestList
14:54:37     INFO -      mp.read(self.manifest)
14:54:37     INFO -    File "/builds/slave/test/build/venv/local/lib/python2.7/site-packages/manifestparser/manifestparser.py", line 536, in read
14:54:37     INFO -      self._read(here, filename, defaults)
14:54:37     INFO -    File "/builds/slave/test/build/venv/local/lib/python2.7/site-packages/manifestparser/manifestparser.py", line 458, in _read
14:54:37     INFO -      raise IOError(message)
14:54:37     INFO -  IOError: Included file '/builds/slave/test/build/tests/xpcshell/tests/toolkit/devtools/debugger/tests/unit/xpcshell.ini' does not exist
14:54:37     INFO -  pushing /system/bin/busybox
14:54:37     INFO -  waiting for system-message-listener-ready...
14:54:37     INFO -  done
14:54:37     INFO -  pushing /builds/slave/test/build/tests/xpcshell/tests
14:54:37     INFO -  Automation Error: Exception caught while running tests
14:54:37     INFO - Return code: 1
14:54:37     INFO - dumping logcat
Whiteboard: [fxos:media]
Target Milestone: --- → 1.4 S3 (14mar)
strict changes the parsing semantics of manifests. I think you'll want to change the handling of --manifest to incur an error if the manifest is empty or not found.

Looking at the code in manifestparser.py, it should fail if a does-not-exist filename is passed. Is something filtering via an os.path.exists() before it gets to ManifestParser()? If so, that should be the thing that changes. Invalid paths as arguments should fail fast.
(In reply to Gregory Szorc [:gps] from comment #2)
> strict changes the parsing semantics of manifests. I think you'll want to
> change the handling of --manifest to incur an error if the manifest is empty
> or not found.
> 
> Looking at the code in manifestparser.py, it should fail if a does-not-exist
> filename is passed. Is something filtering via an os.path.exists() before it
> gets to ManifestParser()? If so, that should be the thing that changes.
> Invalid paths as arguments should fail fast.

No, the problem here is that a manifest that was included by a root manifest was moved or deleted. Whoever moved it didn't update the [include:] directive of the root manifest because the job never complained. This means tests could be accidentally disabled and no one would notice.

Is there any reason not to use strict?
The only reason not to use strict would be if manifests in the wild aren't parsing in strict mode. IMO that should be corrected, strict should be made the default mode, and the manifest parser itself drops support for lenient parsing.

According to bug 902609, xpcshell manifests should be strict compliant.

And, moz.build processes all test manifests in strict mode. So perhaps we are already strict mode compliant everywhere and downstream consumers can change!
Target Milestone: 1.4 S3 (14mar) → 1.5 S3 (6june)
Target Milestone: 2.0 S3 (6june) → 2.0 S4 (20june)
Not working on FxOS any more, so removing myself as assignee
Assignee: dhylands → nobody
You need to log in before you can comment on or make changes to this bug.