Skip to content

Fix regression caused by #25141 - #25153

Merged
juj merged 2 commits into
emscripten-core:mainfrom
juj:fix_parallel_test_runner_regression
Sep 3, 2025
Merged

juj merged 2 commits into
emscripten-core:mainfrom
juj:fix_parallel_test_runner_regression

Conversation

@juj

@juj juj commented Sep 3, 2025

Copy link
Copy Markdown
Collaborator

@juj
juj requested a review from brendandahl September 3, 2025 18:00
@sbc100

sbc100 commented Sep 3, 2025

Copy link
Copy Markdown
Collaborator

I think the issue was that #25141 and #25066 raced and were never tested together.

#25141 removed the last reference to multiprocessing but then #25066 added a fresh one

Comment thread test/common.py Outdated
import json
import logging
import multiprocessing
import threading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line needs to be removed. Looks like I removed the wrong line. Confused how my pr passed though...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is because CircleCI tests PR branches as-is without merging to main.

So a PR on top of main can pass, but then main gets other PRs landed in the meanwhile (like my PR that added use of multithreading).

Individually both PRs get a green stamp on CircleCI, but when merged together, they are not compatible.

@juj

juj commented Sep 3, 2025

Copy link
Copy Markdown
Collaborator Author

The regression is e.g. here: http://clbri.com:8010/#/builders/11/builds/412

@juj
juj merged commit 19e295d into emscripten-core:main Sep 3, 2025
10 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants