Repository navigation
test-runner: run in random order #48385
Description
Activity
Can I take this?
Sure
And I checked that there isn't a good random shuffle function in the codebase yet (CMIIW, I was searching with keyword: shuffle, fisher, yates, ...)
So I think maybe we want to implement the Fisher-Yates shuffle as well? Assuming the shuffling is done offline (after all tests have been enqueued)Assuming the shuffling is done offline (after all tests have been enqueued)
Just FYI - the test runner tries to execute tests as quickly as possible, while respecting the defined concurrency limit. It's pretty unlikely that all of the tests in a file will ever be enqueued.
Just FYI - the test runner tries to execute tests as quickly as possible, while respecting the defined concurrency limit. It's pretty unlikely that all of the tests in a file will ever be enqueued.
I think it's ok to change the behavior in case randomization is on.
Also changing the tree root to be a suite instead of a test would achieve thatI think it's ok to change the behavior in case randomization is on.
As long as it doesn't impact performance, sure.
Also changing the tree root to be a suite instead of a test would achieve that
Wouldn't that only apply to top level tests?
Wouldn't that only apply to top level tests?
Yes, but isn't that what we desire to randomize?
Good question. If I was opting into a randomization mode, I would expect subtests to be randomized as well, but maybe that's a bad assumption on my part?
Or in my part :)
This PR might be helpful to figure out where the internal queue is manged, and where it should be tweaked in case we want to randomize it: #48428
I spent most of the weekend figuring out the best approach to randomize test. I thought to enqueue the test and return a deferred promise here (if randomize is on)
node/lib/internal/test_runner/test.js
Lines 124 to 129 in bd7a808
Then check in the postRun of the
<root>if there are enqueued tests, randomize top level and sub level test using a Fisher-Yates shuffle like thisFor the moment it is only theory, any suggestions are welcome :)
I thought to enqueue the test and return a deferred promise here
that will only cover subtests created with
t.test(wheretis a testContext). there are some other ways to create tests.
howevertest.start()is a method all tests go through after they are created (and it returns a deferred promise)Then check in the postRun of the
<root>if there are enqueued tests,That will break a lot of assumptions and behaviors -
postRunof the root test happens when the event loop is empty - so it is assumed to happen after all tests are completed so it cancels all pending tests.however test.start() is a method all tests go through after they are created (and it returns a deferred promise)
great
That will break a lot of assumptions and behaviors - postRun of the root test happens when the event loop is empty - so it is assumed to happen after all tests are completed so it cancels all pending tests.
I assumed that randomization was always for all the tests, not just for some of them, and when it's time for the
postRunof therootthere are no completed or pending tests, only enqueued tests to be randomized and runned (for the first time).Anyway thanks for the hint.
well, if in the case of randomization is on - we want all tests to be enqueued then shuffled, and only then run - we need to answer the question, when is the latest a test can be enqueued?
a naive approach would be to set the rootsconcurrencyto 0 and then within asetImmediatechange it to the default value, but that would only cover tests defined synchronously. any root test declared after that (with top-level await for example) will simply be enqueued at the end of the queue@MoLow child test should also run in random order, basically ALL tests in one random describe block run in random order, here is the example of vitest random test run, number is order in which test were run:
describe("test1", () => { it("3", () => {}); it("1", () => {}); it("2", () => {}); describe("test2", () => { it("8", () => {}); it("7", () => {}); }); describe("test3", () => { it("6", () => {}); it("4", () => {}); it("5", () => {}); }); });Second run:
describe("test1", () => { it("6", () => {}); it("7", () => {}); it("8", () => {}); describe("test2", () => { it("1", () => {}); it("2", () => {}); }); describe("test3", () => { it("3", () => {}); it("4", () => {}); it("5", () => {}); }); });Reacted by Moshe Atlow and HinataKah0a naive approach would be to set the roots concurrency to 0 and then within a setImmediate change it to the default value
I think we can enqueue all children if user provides
--randomizeflag then let concurrency takes over inprocessPendingSubtests. I believe this is consistent with Jest's randomize behavior as well and it seems that Vitest also behaves the same (from above comment). 👀In the past two weeks, I've been trying to figure out the best way to randomize the tests, and I've experimented with various solutions without much success. When it comes to the
Suite, I managed to randomize all the children, but for theTest, I can only do so if all the children are synchronous, which I believe is not the correct approach.I haven't opened the pull request yet because I wanted to make sure I was heading in the right direction first. I've made two commits on my branch, one for the suite and the other for the test.
At the moment, randomization works for the following cases:
Suites
describe('A1', { shuffle: 123 }, (t) => { it('A1-1', (t) => { assert.strictEqual(1, 1); }); it('A1-2', (t) => { assert.strictEqual(1, 1); }); it('A1-3', (t) => { assert.strictEqual(1, 1); }); it('A1-4', (t) => { assert.strictEqual(1, 1); }); });Test
t.test('A1', { shuffle: 123 }, (t) => { t.test('A1-1', (t) => { assert.strictEqual(1, 1); }); t.test('A1-2', (t) => { assert.strictEqual(1, 1); }); t.test('A1-3', (t) => { assert.strictEqual(1, 1); }); t.test('A1-4', (t) => { assert.strictEqual(1, 1); }); });Both of them always run the child tests in the same order:
A1-3,A1-2,A1-1,A1-4. If theshuffleoption istrue, tests run in a non predictable rundom orderThe problem is that in the case of the
Test, if the children are asynchronous, they fail, so the following doesn't work:t.test('A1', { shuffle: 123 }, async (t) => { await t.test('A1-1', (t) => { assert.strictEqual(1, 1); }); await t.test('A1-2', (t) => { assert.strictEqual(1, 1); }); });There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.
Reacted by Tanguy Krotoff and Toni VillenaThere has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
For more information on how the project manages feature requests, please consult the feature request management document.
Reacted by Tanguy Krotoff
What is the problem this feature will solve?
This feature will improve user tests quality by allowing to run them in random order to ensure that tests are properly isolated from each other.
What is the feature you are proposing to solve the problem?
Allow to run tests in random order based on random seed. See jest randomize or vitest shuffle
Seed will allow to rerun test in the same order to reproduce failed case for specific order.