Open Bug 1223731 Opened 4 years ago Updated 4 years ago
allow URL parameters to be passed to mochitests
I think it might be useful to be able to add URL parameters to mochitests in the manifest, so that a single test file could easily be executed multiple times with different parameters being passed in. One application for this would be tests such as layout/style/test/test_property_syntax_errors.html, which is hitting frequent timeouts on B2G Emulator (bug 925588) because of the number of entries it iterates over in gCSSProperties. With a URL-parameters feature, the test could easily be adapted to run in multiple chunks; then just by writing something like test_property_syntax_errors.html?chunk=1&totalChunks=4 test_property_syntax_errors.html?chunk=2&totalChunks=4 test_property_syntax_errors.html?chunk=3&totalChunks=4 test_property_syntax_errors.html?chunk=4&totalChunks=4 in the manifest, we could avoid any individual test taking so long that it risks hitting the timeout. The same principle would apply to a number of style-system tests that are currently disabled on android/b2g because they hit OOM; by running them in multiple fragments, this could probably be mitigated. Because this test (along with a bunch of others in this directory) is based on the complete collection of properties listed in property_database.js, manually splitting into separate test files would be considerably less convenient.
I've experimented with a hack to do this locally, and as expected, it adds a little bit to the overall test runtime (because it results in loading and iterating over the complete property database multiple times, although on each run only a subset of the entries are actually tested), but the difference in desktop Firefox is pretty trivial (perhaps a few seconds added to the overall test run). The benefit is that it would make it easy for us to enable a bunch of style-system tests that are currently being completely skipped on the mobile platforms. :dholbert, I see you've looked at some of these issues; do you think a solution like this would be helpful?
For the specific case of test_property_syntax_errors.html, I think we might just need requestLongerTimeout() to get longer than the default 300-seconds-per-test timeout, as noted in bug 925588 comment 27 - 29. However, I think the strategy you describe here makes sense, for at least tests that use requestLongerTimeout(4) which makes them run up against the 1000-second "no output detected by harness" timeout (as discussed in bug 1187038). It might be a good strategy for tests that hit OOM, too; though it kinda depends on the reason for the OOM, I guess. (If we're OOM'ing just because we're never letting the GC run somehow, or we're adding nodes continuously without ever dropping them, then I'd imagine we could address that more directly & more simply by doing some periodic cleanup in the test.)
[dbaron will probaby have thoughts here as well; CC'ing him so this is on his radar at least]
You're right about test_property_syntax_errors (bug 925588), that wasn't actually this kind of timeout. Still, I think the ability to easily run other large style-system tests such as test_value_cloning and test_value_calculation in multiple chunks would make it easier for us to re-enable them where they're currently skipped due to "silent-test" timeout and/or OOM issues. I've been able to do this locally with a few hacks to the build and test system code, but pushing it to tryserver currently fails: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7443e428163. Probably there are some additional places where the packaging or test-running code needs to be told to ignore a query on the end of the URL. If this is worth pursuing, we should get people who know about mozbuild and test-runner code to take a look and fix things up properly, instead of my ad hoc hacks.
You need to log in before you can comment on or make changes to this bug.