Conversation
This comment has been minimized.
This comment has been minimized.
|
In this PR random worker-executor tests were seemingly timing out more frequently than before. I've analysed the logs of these cases, fixed optional tracing support, analysed whole test suite runs with otlp traces, etc and the only conclusion I could get is that it depends on when the test-components are first get compiled - as the compilation itself uses all cores and can be relatively long in debug builds for large components like the TS ones. I've measured the wasmtime v33 vs v42 compilation performance and checked if they changed anything in their cache implementation but there should not be any regression so my conclusion is that the "more flaky" behavior is just a random coincidence of things running slightly differently with the new engine. So I've made two changes for worker executor tests (integration tests remain as they are - they test the real behavior where compilation happens when and how it would in production):
This makes the worker-executor tests much more deterministic because they no longer depend on injecting minutes-long compilations in their execution based on the execution order and parallelism. Everything is pre-initialized for them - while test-r's lazy test-dep evaluation guarantees that we can still run just a subset of the tests without penalties. |
|
|
||
| /// Creates a `StoreComponentBuilder` from a `PrecompiledComponent`, automatically | ||
| /// setting both the WASM file name and the package name. | ||
| fn component_dep( |
There was a problem hiding this comment.
Can we make this specific to the worker-executor test dsl? This one doesn't make sense for the integration tests from what I can tell.
There was a problem hiding this comment.
I was thinking maybe it makes sense for them too (but did not do any such change) to have a central definition of the test components and refer to them with test_dep instead of hardcoding their names everywhere. (The precompilation itself should not happen there, of course)
Resolves #2340