Skip to content

Comments

fix(rolldown_test): refine injection logic#5639

Closed
situ2001 wants to merge 4 commits intorolldown:mainfrom
situ2001:situ/wait-for-hmr-obj-ready
Closed

fix(rolldown_test): refine injection logic#5639
situ2001 wants to merge 4 commits intorolldown:mainfrom
situ2001:situ/wait-for-hmr-obj-ready

Conversation

@situ2001
Copy link
Contributor

@situ2001 situ2001 commented Aug 6, 2025

Injection should poll for __rolldown_runtime__ before acutally running test. Otherwise, an error will occur in my Linux x64 machine.

failures:

---- fixture_with_config_tests__rolldown__topics__hmr__dynamic_import__config_json stdout ----
Input: /home/situ/Codes/rolldown/crates/rolldown/tests/rolldown/topics/hmr/dynamic_import/_config.json
⬇️⬇️ Failed to execute command  ⬇️⬇️
"node" "--import" "data:text/javascript,globalThis.__testPatches%20%3D%20%5B%22.%2F1754486755140.js%22%5D%3B%0Aimport%20url%20from%20%27node%3Aurl%27%3B%0Aimport%20path%20from%20%27node%3Apath%27%3B%0A%0Aconst%20dir%20%3D%20%27%2Fhome%2Fsitu%2FCodes%2Frolldown%2Fcrates%2Frolldown%2Ftests%2Frolldown%2Ftopics%2Fhmr%2Fdynamic_import%2Fhmr-temp%2Fdist%27%3B%0AsetTimeout%28async%20%28%29%20%3D%3E%20%7B%0A%20%20for%20%28const%20patchChunk%20of%20globalThis.__testPatches%29%20%7B%0A%20%20%20%20const%20file%20%3D%20path.join%28dir%2C%20patchChunk%29%3B%0A%20%20%20%20try%20%7B%0A%20%20%20%20%20%20await%20import%28url.pathToFileURL%28file%29%29%3B%0A%20%20%20%20%7D%20catch%20%28error%29%20%7B%0A%20%20%20%20%20%20console.error%28%27Error%20executing%20a%20patch%3A%27%2C%20error%29%3B%0A%20%20%20%20%20%20process.exitCode%20%3D%201%3B%0A%20%20%20%20%20%20break%3B%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%2C%2010%29%3B%0A%20%20%20%20%20%20" "--import" "/home/situ/Codes/rolldown/crates/rolldown/tests/rolldown/topics/hmr/dynamic_import/hmr-temp/dist/main.js" "--eval" "\"\"" "--input-type=module"
⬆️⬆️ end  ⬆️⬆️

thread 'fixture_with_config_tests__rolldown__topics__hmr__dynamic_import__config_json' panicked at crates/rolldown_testing/src/integration_test.rs:678:7:
⬇️⬇️ stderr  ⬇️⬇️
Error executing a patch: ReferenceError: __rolldown_runtime__ is not defined
    at file:///home/situ/Codes/rolldown/crates/rolldown/tests/rolldown/topics/hmr/dynamic_import/hmr-temp/dist/1754486755140.js:2:31
    at ModuleJob.run (node:internal/modules/esm/module_job:271:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:578:26)
    at async Timeout._onTimeout (data:text/javascript,globalThis.__testPatches%20%3D%20%5B%22.%2F1754486755140.js%22%5D%3B%0Aimport%20url%20from%20%27node%3Aurl%27%3B%0Aimport%20path%20from%20%27node%3Apath%27%3B%0A%0Aconst%20dir%20%3D%20%27%2Fhome%2Fsitu%2FCodes%2Frolldown%2Fcrates%2Frolldown%2Ftests%2Frolldown%2Ftopics%2Fhmr%2Fdynamic_import%2Fhmr-temp%2Fdist%27%3B%0AsetTimeout%28async%20%28%29%20%3D%3E%20%7B%0A%20%20for%20%28const%20patchChunk%20of%20globalThis.__testPatches%29%20%7B%0A%20%20%20%20const%20file%20%3D%20path.join%28dir%2C%20patchChunk%29%3B%0A%20%20%20%20try%20%7B%0A%20%20%20%20%20%20await%20import%28url.pathToFileURL%28file%29%29%3B%0A%20%20%20%20%7D%20catch%20%28error%29%20%7B%0A%20%20%20%20%20%20console.error%28%27Error%20executing%20a%20patch%3A%27%2C%20error%29%3B%0A%20%20%20%20%20%20process.exitCode%20%3D%201%3B%0A%20%20%20%20%20%20break%3B%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%2C%2010%29%3B%0A%20%20%20%20%20%20:10:7)

⬇️⬇️ stdout ⬇️⬇️

⬆️⬆️ end  ⬆️⬆️
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- fixture_with_config_tests__rolldown__topics__hmr__generate_patch_error__config_json stdout ----
Input: /home/situ/Codes/rolldown/crates/rolldown/tests/rolldown/topics/hmr/generate_patch_error/_config.json
⬇️⬇️ Failed to execute command  ⬇️⬇️
"node" "--import" "data:text/javascript,globalThis.__testPatches%20%3D%20%5B%22.%2F1754486755153.js%22%5D%3B%0Aimport%20url%20from%20%27node%3Aurl%27%3B%0Aimport%20path%20from%20%27node%3Apath%27%3B%0A%0Aconst%20dir%20%3D%20%27%2Fhome%2Fsitu%2FCodes%2Frolldown%2Fcrates%2Frolldown%2Ftests%2Frolldown%2Ftopics%2Fhmr%2Fgenerate_patch_error%2Fhmr-temp%2Fdist%27%3B%0AsetTimeout%28async%20%28%29%20%3D%3E%20%7B%0A%20%20for%20%28const%20patchChunk%20of%20globalThis.__testPatches%29%20%7B%0A%20%20%20%20const%20file%20%3D%20path.join%28dir%2C%20patchChunk%29%3B%0A%20%20%20%20try%20%7B%0A%20%20%20%20%20%20await%20import%28url.pathToFileURL%28file%29%29%3B%0A%20%20%20%20%7D%20catch%20%28error%29%20%7B%0A%20%20%20%20%20%20console.error%28%27Error%20executing%20a%20patch%3A%27%2C%20error%29%3B%0A%20%20%20%20%20%20process.exitCode%20%3D%201%3B%0A%20%20%20%20%20%20break%3B%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%2C%2010%29%3B%0A%20%20%20%20%20%20" "--import" "/home/situ/Codes/rolldown/crates/rolldown/tests/rolldown/topics/hmr/generate_patch_error/hmr-temp/dist/main.js" "--eval" "\"\"" "--input-type=module"
⬆️⬆️ end  ⬆️⬆️

thread 'fixture_with_config_tests__rolldown__topics__hmr__generate_patch_error__config_json' panicked at crates/rolldown_testing/src/integration_test.rs:678:7:
⬇️⬇️ stderr  ⬇️⬇️
Error executing a patch: ReferenceError: __rolldown_runtime__ is not defined
    at file:///home/situ/Codes/rolldown/crates/rolldown/tests/rolldown/topics/hmr/generate_patch_error/hmr-temp/dist/1754486755153.js:2:18
    at ModuleJob.run (node:internal/modules/esm/module_job:271:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:578:26)
    at async Timeout._onTimeout (data:text/javascript,globalThis.__testPatches%20%3D%20%5B%22.%2F1754486755153.js%22%5D%3B%0Aimport%20url%20from%20%27node%3Aurl%27%3B%0Aimport%20path%20from%20%27node%3Apath%27%3B%0A%0Aconst%20dir%20%3D%20%27%2Fhome%2Fsitu%2FCodes%2Frolldown%2Fcrates%2Frolldown%2Ftests%2Frolldown%2Ftopics%2Fhmr%2Fgenerate_patch_error%2Fhmr-temp%2Fdist%27%3B%0AsetTimeout%28async%20%28%29%20%3D%3E%20%7B%0A%20%20for%20%28const%20patchChunk%20of%20globalThis.__testPatches%29%20%7B%0A%20%20%20%20const%20file%20%3D%20path.join%28dir%2C%20patchChunk%29%3B%0A%20%20%20%20try%20%7B%0A%20%20%20%20%20%20await%20import%28url.pathToFileURL%28file%29%29%3B%0A%20%20%20%20%7D%20catch%20%28error%29%20%7B%0A%20%20%20%20%20%20console.error%28%27Error%20executing%20a%20patch%3A%27%2C%20error%29%3B%0A%20%20%20%20%20%20process.exitCode%20%3D%201%3B%0A%20%20%20%20%20%20break%3B%0A%20%20%20%20%7D%0A%20%20%7D%0A%7D%2C%2010%29%3B%0A%20%20%20%20%20%20:10:7)

⬇️⬇️ stdout ⬇️⬇️
.hmr hello
.app hello

⬆️⬆️ end  ⬆️⬆️


failures:
    fixture_with_config_tests__rolldown__topics__hmr__dynamic_import__config_json
    fixture_with_config_tests__rolldown__topics__hmr__generate_patch_error__config_json

test result: FAILED. 532 passed; 2 failed; 4 ignored; 0 measured; 0 filtered out; finished in 28.07s

@hyf0 hyf0 requested a review from sapphi-red August 6, 2025 13:38
graphite-app[bot]
graphite-app bot reviewed Aug 6, 2025
View reviewed changes
hyf0
hyf0 reviewed Aug 6, 2025
View reviewed changes
@hyf0
Copy link
Member

hyf0 commented Aug 6, 2025

@situ2001 Could you confirm if it solves your problem?

@situ2001
Copy link
Contributor Author

situ2001 commented Aug 6, 2025

@situ2001 Could you confirm if it solves your problem?

I have manually run just test for several times using setTimeout(fn, 100) and no error occurred.

But I think it is not a robust solution as it just delays the patching function by using a fixed timeout. If the main module initializes __rolldown_runtime__ after 100ms, the patching will fail too.

@situ2001
Copy link
Contributor Author

situ2001 commented Aug 6, 2025 •
edited
Loading

It's not the only case. I have also investigated the to partially failed action of main branch.

https://github.com/rolldown/rolldown/actions/runs/16769788259/job/47482131558
https://github.com/rolldown/rolldown/actions/runs/16770010071/job/47482789041

Both of them are caused by this issue.

@situ2001 situ2001 requested a review from hyf0 August 6, 2025 16:19
sapphi-red
sapphi-red previously approved these changes Aug 6, 2025
View reviewed changes
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

I wonder why the callback of setTimeout is called before __rolldown_runtime__ is defined. But I think this is good as a temporary solution.

@sapphi-red sapphi-red mentioned this pull request Aug 6, 2025
Merged
@shulaoda shulaoda added this pull request to the merge queue Aug 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 6, 2025
@situ2001 situ2001 mentioned this pull request Aug 7, 2025
Closed
github-merge-queue bot pushed a commit that referenced this pull request Aug 7, 2025
@sapphi-red
test(hmr): run patch exectution after the normal execution (#5645)
2a91c8d 
Alternative to #5639. This worked for me.

Probably, `--import` tries to run not just the first macro task but also
some macro tasks.

@situ2001 Would you try if this one works for you as well?

close #5639 close #5648
@hyf0 hyf0 closed this in #5645 Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@graphite-app graphite-app[bot] graphite-app[bot] left review comments

@sapphi-red sapphi-red sapphi-red left review comments

@hyf0 hyf0 Awaiting requested review from hyf0

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@situ2001 @hyf0 @sapphi-red