Support for shared per-task setup and teardown in tests
Categories
(Testing :: General, enhancement)
Tracking
(firefox140 fixed)
| Tracking | Status | |
|---|---|---|
| firefox140 | --- | fixed |
People
(Reporter: beth, Assigned: beth)
References
(Blocks 3 open bugs)
Details
Attachments
(1 file, 2 obsolete files)
We don't really have a good story right now across the tree for setup/teardown of individual tasks in a test. We have head files, add_setup, and registerCleanupFunction, but these work at the scale of entire tests/sets of tasks.
I currently find myself using the following pattern in Nimbus tests:
function setupTest() {
// common test code
return {
cleanup: () => { /* cleanup code */ },
/* other utilities that are used in the test */
}
}
add_task(function test_foo() {
const { cleanup } = setupTest();
/* do the actual test */
cleanup();
});
Here (1, 2) are some examples of tests doing just that.
NormandyTestUtils exposes a decorate_task method uses the decorator pattern to extract setup and teardown that is common across functions. The decorator pattern looks roughly like this:
function setupTest(fn) {
return async function(ctx) {
/* do the test setup */
try {
await fn(ctx);
} finally {
/* do test setup */
}
};
}
decorate_task(setupTest, function test(ctx) {
/* actual test, no setup/teardown gunk */
});
Normandy server is dead and so Normandy client is slated to to be removed soon, so I propose that we add this functionality to SimpleTest.js and testing/xpcshell/head.js so that we can use this everywhere across the tree.
|
Assignee | |
Comment 1•7 months ago
|
||
This adds a new utility to use decorators to support individual task
setup and teardown.
|
||
Updated•7 months ago
|
|
|
Assignee | |
Comment 2•7 months ago
|
||
I've attached some work in progress patches on top of the linked patch to show what these changes look like in real tests.
|
||
Comment 3•7 months ago
|
||
Thank you for raising this. I agree that having something that applies to individual tests is needed.
Is there a reason to prefer using decorators rather than beforeEach/afterEach?
I've long wondered if we should migrate our testing infrastructure more towards the style of testing frameworks of mocha and Jest for a couple of reasons. One reason would be for familiarity for developers from relevant backgrounds - using beforeEach/afterEach would match those frameworks, but maybe there's other popular frameworks that use the decorator style instead? Another reason is that I wonder if we could eventually switch to using a framework completely, rather than spinning our own. I'm not sure how practical it would be, but it would be easier if the tests we have were in the same style as other frameworks.
One thing I don't like about the decorator style is that each test function must be manually annotated with the decorator - that's adding more boilerplate to the test code which we generally try to avoid. On the contrary, the beforeEach/afterEach can be specified at the top of the file and then "forgotten" about. Whilst we then wouldn't have the option for only some tests in the file to not have those applied, I think we generally don't need that as we separate out tests into different files quite a bit.
The main downside I see with beforeEach/afterEach is that it would be slightly harder to provide common utility functions such as this one, but I think you could do that by having a function call which accepts beforeEach/afterEach as functions.
|
|
Assignee | |
Comment 4•7 months ago
|
||
(In reply to Mark Banner (:standard8) from comment #3)
Thank you for raising this. I agree that having something that applies to individual tests is needed.
Is there a reason to prefer using decorators rather than beforeEach/afterEach?
I must admit that decorators is not my first choice for test setup and teardown -- I would much prefer a more full fledged solution ala mocha test with nested suites. However, I wanted to kickstart this conversation with an example of what we already have in tree and what we could be using today.
I've long wondered if we should migrate our testing infrastructure more towards the style of testing frameworks of mocha and Jest for a couple of reasons. One reason would be for familiarity for developers from relevant backgrounds - using beforeEach/afterEach would match those frameworks, but maybe there's other popular frameworks that use the decorator style instead? Another reason is that I wonder if we could eventually switch to using a framework completely, rather than spinning our own. I'm not sure how practical it would be, but it would be easier if the tests we have were in the same style as other frameworks.
I have also had the idea to turn nested mocha-style tests into add_task calls, but I don't know if I have time to implement that correctly. If we settle on something like that, we could also implement it outside of xpcshell/head.js and simpletest.js and have it opt-in like NormandyTestUtils. Using an existing API design like mocha etc. would also make it easier to port tests written in this adaptor to a different test framework wholesale later.
One thing I don't like about the decorator style is that each test function must be manually annotated with the decorator - that's adding more boilerplate to the test code which we generally try to avoid. On the contrary, the beforeEach/afterEach can be specified at the top of the file and then "forgotten" about. Whilst we then wouldn't have the option for only some tests in the file to not have those applied, I think we generally don't need that as we separate out tests into different files quite a bit.
A global beforeEach/afterEach imposes that each test in the file have identical setup and teardown requirements, whereas the decorator pattern does not.
The main downside I see with beforeEach/afterEach is that it would be slightly harder to provide common utility functions such as this one, but I think you could do that by having a function call which accepts beforeEach/afterEach as functions.
I'm not sure what you mean by this last point, could you show a mock example of what you mean?
|
|
Assignee | |
Updated•7 months ago
|
|
|
Assignee | |
Comment 5•6 months ago
•
|
||
Well I managed to nerdsnipe myself into writing a mocha-to-add_task adapter.
|
|
Assignee | |
Comment 6•6 months ago
|
||
This patch adds Mochia, a simple Mocha-like test interface that wraps
add_task. It allows for composable and shareable setup and teardown
functions across test suites and provides a more familiar interface than
add_task et al. that may make it easier for newcomers to Firefox to
write tests.
|
|
||
Comment 7•6 months ago
|
||
I'm sorry I haven't had time to look at this this week, will try and look early next week.
|
|
||
Updated•6 months ago
|
|
|
||
Comment 8•6 months ago
|
||
(In reply to Beth Rennie [:beth] (she/her) from comment #4)
The main downside I see with beforeEach/afterEach is that it would be slightly harder to provide common utility functions such as this one, but I think you could do that by having a function call which accepts beforeEach/afterEach as functions.
I'm not sure what you mean by this last point, could you show a mock example of what you mean?
I withdraw my comment here. That function would be better rewritten in the appropriate style for before/afterEach.
|
||
Comment 10•5 months ago
|
||
|
||
Comment 11•5 months ago
|
||
Backed out for causing several xpcshell failures
Failure logs:
|
|
Assignee | |
Comment 12•5 months ago
|
||
Of all of those tests, test_bug1378385_http1.js seems to be failing for me without this patch applied. The rest seem to pass with or without the patch applied.
|
|
Assignee | |
Comment 13•5 months ago
|
||
I have filed https://bugzilla.mozilla.org/show_bug.cgi?id=1965549 about the suspected perma fail. Gonna retry landing because I am not convinced those failures are correlated with this patch.
|
|
||
Comment 14•5 months ago
|
||
|
|
||
Comment 15•5 months ago
|
||
|
||
Comment 16•5 months ago
•
|
||
Revert "Bug 1957513 - for causing xpcshell failures on test_bug1312782_http1.js"
LATER EDIT: this actually caused multiple failures - more failure examples:
-
https://treeherder.mozilla.org/logviewer?job_id=507484288&repo=autoland&lineNumber=5059
-
https://treeherder.mozilla.org/logviewer?job_id=507485522&repo=autoland&lineNumber=4039
-
https://treeherder.mozilla.org/logviewer?job_id=507485519&repo=autoland&lineNumber=3884
[task 2025-05-09T21:12:35.048Z] 21:12:35 INFO - TEST-START | netwerk/test/unit/test_bug1312782_http1.js
[task 2025-05-09T21:12:35.093Z] 21:12:35 INFO - adb launch_application: am startservice -W -n 'org.mozilla.geckoview.test_runner/org.mozilla.geckoview.test_runner.XpcshellTestRunnerService$i0' -a android.intent.action.MAIN --es env0 XPCOM_DEBUG_BREAK=stack-and-abort --es env1 MOZ_CRASHREPORTER=1 --es env2 MOZ_CRASHREPORTER_NO_REPORT=1 --es env3 MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 --es env4 MOZ_DEVELOPER_REPO_DIR=/builds/worker/checkouts/gecko --es env5 MOZ_DEVELOPER_OBJ_DIR=/builds/worker/workspace/obj-build --es env6 MOZ_DISABLE_CONTENT_SANDBOX=1 --es env7 MOZ_FETCHES_DIR=/builds/worker/fetches --es env8 MOZ_DISABLE_SOCKET_PROCESS=1 --es env9 LD_LIBRARY_PATH=/data/local/tmp/test_root/xpcb --es env10 MOZ_LINKER_CACHE=/data/local/tmp/test_root/xpcb --es env11 GRE_HOME=/data/local/tmp/test_root/xpcb --es env12 XPCSHELL_TEST_PROFILE_DIR=/data/local/tmp/test_root/xpc/p/f1a08c66-218d-4f25-8690-831c6b0c2f01 --es env13 HOME=/data/local/tmp/test_root/xpc/p --es env14 XPCSHELL_TEST_TEMP_DIR=/data/local/tmp/test_root/xpc/tmp/79f2c61b-9712-4683-a104-536a2737835c --es env15 MOZ_ANDROID_DATA_DIR=/data/local/tmp/test_root/xpcb --es env16 MOZ_IN_AUTOMATION=1 --es env17 MOZ_ANDROID_CPU_ABI=x86_64 --es env18 MOZHTTP2_PORT=38175 --es env19 MOZNODE_EXEC_PORT=38812 --es env20 MOZHTTP3_PORT=42942 --es env21 MOZHTTP3_PORT_FAILED=42013 --es env22 MOZHTTP3_PORT_ECH=49233 --es env23 MOZHTTP3_PORT_PROXY=48536 --es env24 MOZHTTP3_PORT_NO_RESPONSE=58755 --es env25 MOZHTTP3_ECH=AE3+DQBJBwAgACCKoGbpR/+g/R5G0gvkgPHW7Lp25XqiToguiY6AVcY5LwAQAAEAAQABAAMAAgABAAIAA0AOcHVibGljLmV4YW1wbGUAAA== --es env26 MOZ_HTTP3_SERVER_PATH=/builds/worker/fetches/hostutils/http3server --es env27 MOZ_HTTP3_CERT_DB_PATH=/builds/worker/workspace/build/tests/xpcshell/http3server/http3serverDB --es env28 TMPDIR=/data/local/tmp/test_root/xpc/p/f1a08c66-218d-4f25-8690-831c6b0c2f01 --es env29 XPCSHELL_MINIDUMP_DIR=/data/local/tmp/test_root/xpc/minidumps/f1a08c66-218d-4f25-8690-831c6b0c2f01 --es arg0 -g --es arg1 /data/local/tmp/test_root/xpcb --es arg2 --greomni --es arg3 /data/local/tmp/test_root/xpcb/geckoview-test_runner.apk --es arg4 -m --es arg5 -e --es arg6 'const _HEAD_JS_PATH = "/data/local/tmp/test_root/xpc/head.js";' --es arg7 -e --es arg8 'const _MOZINFO_JS_PATH = "/data/local/tmp/test_root/xpc/p/f1a08c66-218d-4f25-8690-831c6b0c2f01/mozinfo.json";' --es arg9 -e --es arg10 'const _PREFS_FILE = "/data/local/tmp/test_root/xpc/tmp/79f2c61b-9712-4683-a104-536a2737835c/user.js";' --es arg11 -e --es arg12 'const _TESTING_MODULES_DIR = "/data/local/tmp/test_root/xpc/m";' --es arg13 -f --es arg14 /data/local/tmp/test_root/xpc/head.js --es arg15 -e --es arg16 'const _HEAD_FILES = ["/data/local/tmp/test_root/xpc/netwerk/test/unit/head_channels.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_cache.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_cache2.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_cookies.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_servers.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_trr.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_http3.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_telemetry.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_websocket.js", "/data/local/tmp/test_root/xpc/netwerk/test/unit/head_webtransport.js"];' --es arg17 -e --es arg18 'const _JSDEBUGGER_PORT = 0;' --es arg19 -e --es arg20 'const _TEST_CWD = "/data/local/tmp/test_root/xpc/netwerk/test/unit";' --es arg21 -e --es arg22 'const _TEST_FILE = ["test_bug1312782_http1.js"];' --es arg23 -e --es arg24 'const _TEST_NAME = "netwerk/test/unit/test_bug1312782_http1.js";' --es arg25 -e --es arg26 '_execute_test(); quit(0);' --es out_file /data/local/tmp/test_root/xpc/logs/xpcshell-5018b6bd-ddf1-4ab7-b254-463596be3dfd.log
[task 2025-05-09T21:12:35.312Z] 21:12:35 INFO - remotexpcshelltests.py | netwerk/test/unit/test_bug1312782_http1.js | 12108 | Launched Test App
[task 2025-05-09T21:12:36.378Z] 21:12:36 INFO - remotexpcshelltests.py | netwerk/test/unit/test_bug1312782_http1.js | 12108 | Application ran for: 0:00:01.329105
[task 2025-05-09T21:12:36.441Z] 21:12:36 WARNING - TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_bug1312782_http1.js | xpcshell return code: 0
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - TEST-INFO took 1392ms
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - >>>>>>>
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (2)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (3)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (4)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (5)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (6)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (7)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test MAIN run_test finished (7)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - running event loop
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (7)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (8)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (9)
[task 2025-05-09T21:12:36.441Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (10)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (11)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test pending (12)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (12)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (11)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (10)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (9)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (8)
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (7)
[task 2025-05-09T21:12:36.442Z] 21:12:36 WARNING - TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_bug1312782_http1.js | check_response_id - [check_response_id : 116] false == true
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - test_bug1312782_http1.js:check_response_id:116
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - test_bug1312782_http1.js:setup_http_server/<:159
[task 2025-05-09T21:12:36.442Z] 21:12:36 INFO - resource://testing-common/httpd.sys.mjs:handleResponse:2494
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - resource://testing-common/httpd.sys.mjs:process:1346
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - resource://testing-common/httpd.sys.mjs:_handleResponse:1790
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - resource://testing-common/httpd.sys.mjs:_processBody:1638
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - resource://testing-common/httpd.sys.mjs:onInputStreamReady:1517
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - /data/local/tmp/test_root/xpc/head.js:_do_main:245
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - /data/local/tmp/test_root/xpc/head.js:_execute_test:596
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - -e:null:1
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - exiting test
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - "Force a GC"
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (6)
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (5)
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (4)
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (3)
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (2)
[task 2025-05-09T21:12:36.443Z] 21:12:36 INFO - (xpcshell/head.js) | test finished (1)
[task 2025-05-09T21:12:36.444Z] 21:12:36 INFO - exiting test
[task 2025-05-09T21:12:36.444Z] 21:12:36 INFO - <<<<<<<
[task 2025-05-09T21:12:36.512Z] 21:12:36 INFO - Cleaning up profile for /builds/worker/workspace/build/tests/xpcshell/tests/netwerk/test/unit/test_bug1312782_http1.js folder: /data/local/tmp/test_root/xpc/p/f1a08c66-218d-4f25-8690-831c6b0c2f01
[task 2025-05-09T21:12:37.149Z] 21:12:37 INFO - TEST-START | netwerk/test/unit/test_http2_with_proxy.js
|
|
Assignee | |
Comment 17•5 months ago
|
||
|
|
Assignee | |
Updated•5 months ago
|
|
|
Assignee | |
Updated•5 months ago
|
|
|
||
Comment 18•5 months ago
|
||
|
||
Comment 19•5 months ago
|
||
| bugherder | ||
|
|
Assignee | |
Comment 20•5 months ago
|
||
The first part of this with support for mochitest has landed so im going to split the rest into bug 1969102
|
|
||
Comment 21•5 months ago
|
||
Comment on attachment 9487397 [details]
Bug 1957513 - Add Mochia to xpcshell tests r?standard8!,gijs!
Revision D249125 was moved to bug 1969102. Setting attachment 9487397 [details] to obsolete.
|
|
Assignee | |
Updated•5 months ago
|
|
||
Updated•5 months ago
|
Description
•