test: speed up "create and sync data" test #689
Conversation
*I recommend reviewing this with whitespace changes disabled.* This speeds up our "create and sync data" test. On my machine, it went from ~73 seconds to ~12. I did this by making a few changes: - Decrease the number of managers from 10 to 5, dramatically reducing the amount of connections required. I don't think 10 really offers much here, other than perhaps a stress test, and lowering it offers a huge speedup. - Run assertions concurrently. - When comparing sets, use `Set` instead of sorted arrays. This speeds things up a bit. - Use `node:assert` instead of Brittle for a slight performance boost. We also plan to drop Brittle in the future and this will ease the transition. Each of these could theoretically be done separately. I've been running this test a lot which is why I'm fixing this now.
test-e2e/sync.js
Outdated
| Promise.all( | ||
| map(projects, async (project) => { | ||
| const deviceId = project.deviceId.slice(0, 7) | ||
| // @ts-ignore - to complex to narrow `schemaName` to valid values |
There was a problem hiding this comment.
nit: may as well fix the grammar 😄
| // @ts-ignore - to complex to narrow `schemaName` to valid values | |
| // @ts-ignore - too complex to narrow `schemaName` to valid values |
test-e2e/sync.js
Outdated
| // NOTE: Unlike other tests in this file, this test uses `node:assert` instead | ||
| // of `t` to ease our transition away from Brittle. We can remove this comment | ||
| // when Brittle is removed. | ||
| const COUNT = 5 |
There was a problem hiding this comment.
having this stay higher to stress test seems generally useful, but maybe there's an alternative approach to achieve that? i know we introduced something like a FAST_TESTS env variable but i don't think it's being used 🤔
There was a problem hiding this comment.
Nah, I think it's useful too. Reverted to 10 in 340bf82. I might consider using FAST_TESTS in the future, but not necessary now.
There was a problem hiding this comment.
curious how much faster the test is now for you, since the only thing related change is parallelizing the assertions?
EDIT: the major thing changed is parallelizing the assertions. there are other things as mentioned that probably help a bit too
There was a problem hiding this comment.
It sped things up by about 10 seconds on my machine.
*This is a test-only change.* I've been working on sync lately, and this test takes a long time. This change speeds it up if the `FAST_TESTS` environment variable is set. Stole this [idea from Andrew][0]. [0]: #689 (comment)
*This is a test-only change.* I've been working on sync lately, and this test takes a long time. This change speeds it up if the `FAST_TESTS` environment variable is set. Stole this [idea from Andrew][0]. [0]: #689 (comment)
I recommend reviewing this with whitespace changes disabled.
This speeds up our "create and sync data" test. On my machine, it went from ~73 seconds to ~12.
I did this by making a few changes:
Decrease the number of managers from 10 to 5, dramatically reducing the amount of connections required. I don't think 10 really offers much here, other than perhaps a stress test, and lowering it offers a huge speedup.
Run assertions concurrently.
When comparing sets, use
Setinstead of sorted arrays. This speeds things up a bit.Use
node:assertinstead of Brittle for a slight performance boost. We also plan to drop Brittle in the future and this will ease the transition.Each of these could theoretically be done separately.
I've been running this test a lot which is why I'm fixing this now.