More emscripten test fixes #31623
More emscripten test fixes #31623
Conversation
| let test_result = calc_result(&desc2, test_result); | ||
| let stdout = data3.lock().unwrap().to_vec(); | ||
| monitor_ch2.send((desc2.clone(), test_result, stdout)).unwrap(); | ||
| } |
There was a problem hiding this comment.
Perhaps this could become the way tests are run? It'd be good to exercise this all the time rather than just with emscripten.
There was a problem hiding this comment.
But then tests would execute one at a time instead of concurrently.
There was a problem hiding this comment.
We don't want to reuse threads for tests generally because of TLS.
There was a problem hiding this comment.
It does look like a lot of duplication for a path that won't be exercised much. Can any of it be factored out?
There was a problem hiding this comment.
I don't think it's possible to reduce it, except for some minor things.
It would probably be feasible if Builder::spawn() returned the closure that was supposed to be called in the case of an error.
|
I pushed a second minor commit that finishes ignoring all tests. |
|
Can you link your emscripten changes? Are they likely to be accepted upstream? |
|
I submitted emscripten-core/emscripten#4097 and emscripten-core/emscripten#4095. The PRs may need some tweaks, but I think the nature of the changes is probably good. I also have some preliminary changes that add proper support for unwinding, but it needs more work before I submit it. |
|
@tomaka Can we change the test runner code to use conditional compilation to detect emscripten and run single-threaded? That way we don't have this never-executed failure path on other platforms. |
|
Closing due to inactivity, but feel free to resubmit with a rebase! |
Most of these rely on spawning processes, which is not possible in Emscripten. Based on rust-lang#31623
Emscripten test fixes This picks up parts of rust-lang#31623 to disable certain tests that emscripten can't run, as threads/processes are not supported. I re-applied @tomaka's changes manually, I can rebase those commits with his credentials if he wants. It also disables jemalloc for emscripten (at least in Rustbuild, I have to check if there is another setting for the same thing in the old makefile approach). This should not impact anything for normal builds.
Emscripten test fixes This picks up parts of rust-lang#31623 to disable certain tests that emscripten can't run, as threads/processes are not supported. I re-applied @tomaka's changes manually, I can rebase those commits with his credentials if he wants. It also disables jemalloc for emscripten (at least in Rustbuild, I have to check if there is another setting for the same thing in the old makefile approach). This should not impact anything for normal builds.
r? @brson
libtestnow runs each test in single-threaded mode (aroundpanic::recover) if the test thread fails to spawn. This is required since emscripten doesn't have threads.abort(-1). I think it's better to always error when a symbol is missing.DISABLE_EXCEPTION_CATCHING=0is required to make exceptions work when optimizations are on. However the docs say that this causes a big difference in binary size and runtime performances. From my experience with emscripten, these differences can sometimes be massive.I also have some modifications in my local clone of the emscripten SDK to make all this work.