Skip to content

test(dev): restore runtime assertions of delete_file_not_used_anymore#10112

Merged
graphite-app[bot] merged 1 commit into
mainfrom
07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore
Jul 6, 2026
Merged

test(dev): restore runtime assertions of delete_file_not_used_anymore#10112
graphite-app[bot] merged 1 commit into
mainfrom
07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore

Conversation

@shulaoda

@shulaoda shulaoda commented Jul 3, 2026

Copy link
Copy Markdown
Member

Description

Addresses the review feedback on #10107. Stacked on #10110. Two of the four comments need changes and they live here. The third comment (a failed scan leaves marks that later materialize edge list drift into the HMR importer sets) is fixed by #10110, which reverts all cache mutations of an aborted partial scan. The fourth comment (rescans mark every dependency even when the import set did not change) stays a follow-up, with one note for whoever picks it up: skipping scan output indexes in merge is not a pure win, because the loader refreshes ModuleInfo only for modules with non empty importers and the merge loop currently covers the empty case.

Stale doc references

internal-docs/cache/implementation.md still pointed at three merge call sites in hmr_stage.rs. The site near :621 no longer exists. The Callers section and the writers table rows for merge and update_defer_sync_data now cite the two real HMR call sites (the update path and the lazy compile path).

Restore runtime assertions of delete_file_not_used_anymore

"expectExecuted": false is a fixture wide switch. Only the orphan recreation step needs it, but it also turned off the runtime assertions of the initial build and of every other step, making the whole fixture snapshot only.

The fixture is now split:

  • delete_file_not_used_anymore keeps the executing steps (drop the import, then delete the file) and runs its runtime assertions again.
  • The new hmr/recreate_orphan_module fixture carries the full four step sequence (drop the import, delete the file, recreate it, re-import it) as snapshot only. It documents the current limitation that editing or recreating an orphaned module triggers a full reload until the orphan cleanup tracked in Properly update internal states for incremental builds #7416 lands.

The recreation step cannot stay in the executing fixture, because the re-import step needs the recreated file on disk, so the sequence only makes sense as a whole.

shulaoda commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@shulaoda
shulaoda requested review from h-a-n-a and removed request for IWANABETHATGUY and hyfdev July 3, 2026 08:32
@shulaoda
shulaoda changed the base branch from 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails to graphite-base/10112 July 3, 2026 12:02
@shulaoda
shulaoda force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from a6a617c to 41808d3 Compare July 3, 2026 12:06
@shulaoda
shulaoda force-pushed the graphite-base/10112 branch from 814b555 to 2390768 Compare July 3, 2026 12:06
@shulaoda
shulaoda changed the base branch from graphite-base/10112 to 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails July 3, 2026 12:07
@shulaoda
shulaoda force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from dbc6172 to 41808d3 Compare July 3, 2026 12:14
@codspeed-hq

codspeed-hq Bot commented Jul 3, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore (dbc6172) with 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails (814b555)2

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails (2390768) during the generation of this report, so a610fa4 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@shulaoda
shulaoda changed the base branch from 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails to graphite-base/10112 July 3, 2026 12:18
@shulaoda
shulaoda force-pushed the graphite-base/10112 branch from 2390768 to 9d2b9c6 Compare July 3, 2026 12:21
@shulaoda
shulaoda force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from 41808d3 to 16f06e7 Compare July 3, 2026 12:21
@shulaoda
shulaoda changed the base branch from graphite-base/10112 to 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails July 3, 2026 12:21
@shulaoda
shulaoda force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from 16f06e7 to df2e97a Compare July 3, 2026 12:44
@shulaoda
shulaoda force-pushed the 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails branch from 9d2b9c6 to 2d18156 Compare July 3, 2026 12:44
@graphite-app
graphite-app Bot force-pushed the 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails branch 2 times, most recently from 5c787cc to 00250db Compare July 6, 2026 07:26
@graphite-app
graphite-app Bot force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from df2e97a to adb030b Compare July 6, 2026 07:26

h-a-n-a commented Jul 6, 2026

Copy link
Copy Markdown
Member

Merge activity

  • Jul 6, 8:11 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 6, 8:12 AM UTC: h-a-n-a added this pull request to the Graphite merge queue.
  • Jul 6, 8:18 AM UTC: The Graphite merge queue couldn't merge this PR because it had merge conflicts.
  • Jul 6, 8:35 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 6, 8:36 AM UTC: shulaoda added this pull request to the Graphite merge queue.
  • Jul 6, 8:42 AM UTC: Merged by the Graphite merge queue.

@graphite-app
graphite-app Bot changed the base branch from 07-03-fix_dev_revert_cache_mutations_when_a_partial_scan_fails to graphite-base/10112 July 6, 2026 08:11
@graphite-app
graphite-app Bot changed the base branch from graphite-base/10112 to main July 6, 2026 08:16
@shulaoda
shulaoda force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from adb030b to 08c85ae Compare July 6, 2026 08:27
@netlify

netlify Bot commented Jul 6, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit fc4a92c
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a4b693763503b000808aaff

@shulaoda shulaoda assigned shulaoda and unassigned h-a-n-a Jul 6, 2026
@shulaoda
shulaoda force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from 08c85ae to bbaa42b Compare July 6, 2026 08:32
…#10112)

### Description

Addresses the review feedback on #10107. Stacked on #10110. Two of the four comments need changes and they live here. The third comment (a failed scan leaves marks that later materialize edge list drift into the HMR importer sets) is fixed by #10110, which reverts all cache mutations of an aborted partial scan. The fourth comment (rescans mark every dependency even when the import set did not change) stays a follow-up, with one note for whoever picks it up: skipping scan output indexes in `merge` is not a pure win, because the loader refreshes `ModuleInfo` only for modules with non empty importers and the merge loop currently covers the empty case.

#### Stale doc references

`internal-docs/cache/implementation.md` still pointed at three `merge` call sites in `hmr_stage.rs`. The site near `:621` no longer exists. The `Callers` section and the writers table rows for `merge` and `update_defer_sync_data` now cite the two real HMR call sites (the update path and the lazy compile path).

#### Restore runtime assertions of `delete_file_not_used_anymore`

`"expectExecuted": false` is a fixture wide switch. Only the orphan recreation step needs it, but it also turned off the runtime assertions of the initial build and of every other step, making the whole fixture snapshot only.

The fixture is now split:

- `delete_file_not_used_anymore` keeps the executing steps (drop the import, then delete the file) and runs its runtime assertions again.
- The new `hmr/recreate_orphan_module` fixture carries the full four step sequence (drop the import, delete the file, recreate it, re-import it) as snapshot only. It documents the current limitation that editing or recreating an orphaned module triggers a full reload until the orphan cleanup tracked in #7416 lands.

The recreation step cannot stay in the executing fixture, because the re-import step needs the recreated file on disk, so the sequence only makes sense as a whole.

<!--
- What is this PR solving? Write a clear and concise description.
- Reference the issues it solves (e.g. `fixes #123`).
- What other alternatives have you explored?
- Are there any parts you think require more attention from reviewers?

Also, please make sure you do the following:

- Read the Contributing Guidelines at https://rolldown.rs/contribution-guide/.
- Check that there isn't already a PR that solves the problem the same way. If you find a duplicate, please help us review it.
- Update the corresponding documentation if needed.
- Include relevant tests that fail without this PR but pass with it. If the tests are not included, explain why.

Thank you for contributing to Rolldown!
-->
@graphite-app
graphite-app Bot force-pushed the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch from bbaa42b to fc4a92c Compare July 6, 2026 08:37
@graphite-app
graphite-app Bot merged commit fc4a92c into main Jul 6, 2026
32 of 33 checks passed
@graphite-app
graphite-app Bot deleted the 07-03-test_dev_restore_runtime_assertions_of_delete_file_not_used_anymore branch July 6, 2026 08:42
graphite-app Bot pushed a commit that referenced this pull request Jul 6, 2026
…fter each HMR step (#10115)

### Description

Part of #7416. Stacked on #10112. This adds the test #7416 asks for: one that ensures an incremental build and a full build produce the same states.

#### What it does

After each HMR step of a dev fixture, the test harness now builds a fresh bundler with the same options and plugins against the current file state and runs a full build. The fresh bundler has no history, so its scan state is by definition the correct answer for the files on disk. The harness then asserts the incremental state matches it:

- `importers` and `dynamic_importers` of every module, compared as sets of module ids. Importer entries pointing at modules the fresh build does not know are ignored: those are the outgoing records of orphaned modules, which stay in the state by design (issue #7416). A stale importer entry from a module the fresh build *does* have is still reported.
- `importers_idx`, mapped back to module ids since the two sides use different index spaces, with the same orphan filter.
- Every import record's kind and resolved target.
- Entry points, where the fresh set must be a subset of the incremental set. Extras on the incremental side are allowed for now, because removed modules and stale dynamic import entries are separate open items of #7416.

Any difference fails the test with a per module, per field report.

#### Why per step instead of once at the end

An end of run comparison has a blind spot: a divergence on a cached module is silently repaired as soon as a later step happens to rescan that module, so only the intermediate state is wrong, which is exactly when HMR reads it. This was verified by temporarily reintroducing the bug fixed in #10107 (a cached module keeping a stale importer set). The end of run check passed, the per step check caught it immediately and reported the exact missing importer.

#### When it is skipped

- Steps whose file state does not build, e.g. the error recovery fixtures while their syntax error is in place. A state that cannot be built has no full build to compare against.
- Steps whose incremental build failed. A failed scan is reverted, so the state intentionally stays at the last good build and cannot mirror the current files until a later scan retries them.
- Fixtures with lazy barrel enabled. It loads import records on demand, so the loaded subset depends on request history and legitimately differs between a session and a fresh build.

#### Opting out

`dev.checkStateParity` defaults to `true`, so every existing and future dev fixture gets the check for free (all 38 current HMR fixtures pass with it enabled). A fixture that intentionally documents a known divergence can set it to `false`, and the flag in its config then serves as a visible marker of the known gap until the underlying fix lands.

#### Implementation notes

- The comparison lives inside the `rolldown` crate behind the existing `testing` feature (`Bundler::assert_scan_state_parity_with`), so it can read the private cache directly and no new public API surface is added.
- `DevEngine` gains a `testing` gated `bundler()` accessor, following the existing `get_watched_files` pattern.
- `ensure_task_with_changed_files` now sends the whole step as one watch event batch. One event per message spawned one build per file while the awaited future only covered the first, so a multi file step could race the assertion.
- The config schema is regenerated by the existing build script.

<!--
- What is this PR solving? Write a clear and concise description.
- Reference the issues it solves (e.g. `fixes #123`).
- What other alternatives have you explored?
- Are there any parts you think require more attention from reviewers?

Also, please make sure you do the following:

- Read the Contributing Guidelines at https://rolldown.rs/contribution-guide/.
- Check that there isn't already a PR that solves the problem the same way. If you find a duplicate, please help us review it.
- Update the corresponding documentation if needed.
- Include relevant tests that fail without this PR but pass with it. If the tests are not included, explain why.

Thank you for contributing to Rolldown!
-->
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 8, 2026
shulaoda added a commit that referenced this pull request Jul 8, 2026
## [1.1.5] - 2026-07-08

### 🚀 Features

- detect top-level import-binding reads as execution-order sensitive (#10180) by @hyf0
- sourcemap_filenames: add a sourcemapFileNames option (#9271) by @V1OL3TF0X
- binding: record plugin hook result kind in tracing spans (#10154) by @IWANABETHATGUY
- linking: skip side-effect-free modules in per-entry reachability (#10111) by @IWANABETHATGUY
- improve error message for unresolved virtual imports (#10156) by @sapphi-red
- add descriptive metadata to plugin API (#10106) by @sapphi-red
- add `--configLoader=native` option (#10118) by @sapphi-red

### 🐛 Bug Fixes

- improve invalid annotation warnings (#10185) by @hyf0
- keep deduplicated asset filenames stable once they can be observed (#10191) by @shulaoda
- sourcemap_filenames: use public option name in pattern errors (#10188) by @IWANABETHATGUY
- sourcemap_filenames: hash prepared sourcemap content (#10178) by @hyf0
- tree-shake unused circular declarators exported via export list (#10166) by @IWANABETHATGUY
- dev: don't panic when an HMR rebuild hits an unresolved import (#10162) by @shulaoda
- propagate errors from output.globals function (#9880) by @shulaoda
- dev: revert cache mutations when a partial scan fails (#10110) by @shulaoda
- dev: update importer relationships of cached modules in incremental build (#10107) by @shulaoda
- hmr: fall back to full reload when a changed module is not registered as executed (#10132) by @shulaoda
- chunk-optimizer: follow entry facade edges in runtime placement cycle check (#10101) by @hyf0
- dev: ignore watcher events after close (#10113) by @hyf0
- emit async wrapper for TLA modules under onDemandWrapping (#10086) by @IWANABETHATGUY
- gate sideEffects:false modules' side effects on body demand (#10080) by @IWANABETHATGUY
- rolldown_plugin_vite_resolve: return empty object for `browser: false` mapped modules (#10082) by @sapphi-red
- reset the word-boundary state on newline in Hires::Boundary sourcemaps (#10025) by @shulaoda
- trim an emptied chunk's outro/intro instead of skipping past it (#10029) by @shulaoda
- test each edited chunk's own start against indent exclude ranges (#10026) by @shulaoda
- preserve sourcemap mappings for indented lines when a CJS module shares the chunk (#10074) by @hyf0

### 🚜 Refactor

- separate tree-shaking side effects from execution order sensitivity (#10168) by @hyf0
- type construct_vite_preload_call to take an ObjectPattern (#10135) by @shulaoda
- treeshake: single-source the own-export classification shared with the lazy-barrel loader (#10098) by @IWANABETHATGUY
- dev: reuse Vite's bundledDev server (#10081) by @h-a-n-a
- clippy: ban std HashMap/HashSet in favour of FxHashMap/FxHashSet (#10108) by @Boshen
- treeshake: make body demand a second module bit instead of a stmt multimap (#10097) by @IWANABETHATGUY
- seal used_symbol_refs by construction after its last writer (#10091) by @hyf0
- treeshake: replace inclusion mutual recursion with a worklist engine (#10096) by @IWANABETHATGUY
- treeshake: split include_statements.rs into focused modules (#10095) by @IWANABETHATGUY
- drop redundant is_user_defined filter on partitioned entries (#10050) by @shulaoda
- project the retained export interface out of used_symbol_refs (#10089) by @hyf0
- track used external symbols separately from used_symbol_refs (#10088) by @hyf0
- make module namespace inclusion an explicit linking metadata field (#10087) by @hyf0
- rename statement evaluation metadata (#10078) by @hyf0

### 📚 Documentation

- virtual modules user-facing id convention (#10155) by @sapphi-red
- cli: clarify disabling boolean/object flags like codeSplitting (#10153) by @IWANABETHATGUY
- chore: remove Vite+ alpha banner (#10105) by @mdong1909
- write down the used_symbol_refs contract (#10090) by @hyf0
- dev/lazy: update design and implementation (#10079) by @h-a-n-a

### ⚡ Performance

- ast_scanner: stop order-sensitivity checks once a module is flagged (#10190) by @IWANABETHATGUY
- return impl ExactSizeIterator from slice-backed accessors (#10133) by @Boshen
- binding: box dev and watcher napi futures (#10103) by @Boshen

### 🧪 Testing

- move string_wizard replace unit tests to the JS magic-string suite (#10176) by @IWANABETHATGUY
- dev: assert incremental scan state matches a fresh full build after each HMR step (#10115) by @shulaoda
- dev: restore runtime assertions of delete_file_not_used_anymore (#10112) by @shulaoda
- dev: fix flaky dev server tests in CI (#10152) by @h-a-n-a
- add regression test for #10099 (lazyBarrel drops default-import binding but keeps its property reads) (#10109) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- deploy website to Void via GitHub OIDC (#10192) by @Boshen
- deps: update oxc to 0.139.0 (#10161) by @shulaoda
- deps: update test262 submodule for tests (#10160) by @rolldown-guard[bot]
- rolldown_plugin_utils: remove dead asset-url and css scaffolding (#10131) by @shulaoda
- deps: revert vite-plus to v0.2.1 (#10148) by @shulaoda
- deps: update github actions (#10141) by @renovate[bot]
- deps: update dependency rust to v1.96.1 (#10145) by @renovate[bot]
- deps: update npm packages (#10142) by @renovate[bot]
- deps: update rust crates (#10143) by @renovate[bot]
- deps: update napi to v3.10.3 (#10121) by @renovate[bot]
- rolldown_utils: remove unused time module (#10138) by @shulaoda
- remove dead CopyModulePlugin::is_active method (#10129) by @shulaoda
- remove dead LazyCompilationContext::is_lazy_module method (#10128) by @shulaoda
- remove dead BuildDiagnostic::downcast_ref method (#10127) by @shulaoda
- deps: update dependency vite-plus to v0.2.2 (#10084) by @renovate[bot]
- deps: update rust crate oxc_sourcemap to v8.1.0 (#10122) by @renovate[bot]
- deps: update crate-ci/typos action to v1.48.0 (#10124) by @renovate[bot]
- enable more clippy restriction lints (#10114) by @Boshen
- deps: update rust dependencies (#10100) by @Boshen
- deps: update oxc resolver to v11.23.0 (#10083) by @renovate[bot]

### ◀️ Revert

- Revert "chore(deps): revert vite-plus to v0.2.1" (#10157) by @h-a-n-a
- "fix(hmr): fall back to full reload when a changed module is not registered as executed (#10132)" (#10151) by @shulaoda

### ❤️ New Contributors

* @V1OL3TF0X made their first contribution in [#9271](#9271)

Co-authored-by: shulaoda <[email protected]>
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.

2 participants