Skip to content

Conversation

@rgrinberg
Copy link
Member

@rgrinberg rgrinberg commented Jul 7, 2025

Problem

When running a cram test and then promoting its result, dune will immediately re-run the test to confirm the result is valid. This step is unnecessary when the set of commands does not change after the promotion. This PR avoids the unnecesasry re-running in exactly this case.

How it Works?

We factor out the single cram action into 3 components:

  1. An action that generates the set of commands from the cram test. This corresponds to the script we're going to execute
  2. An action that executes the script in step 1. and produces the output in a serialized form.
  3. An action that diffs the output produced by step 2. against the original script.

@rgrinberg rgrinberg force-pushed the ps/rr/fix_cram___do_not_re_run_tests_unnecessarily branch 6 times, most recently from 1aa13f5 to 46125d9 Compare July 11, 2025 16:51
@rgrinberg rgrinberg marked this pull request as ready for review July 11, 2025 17:06
@rgrinberg rgrinberg force-pushed the ps/rr/fix_cram___do_not_re_run_tests_unnecessarily branch from 46125d9 to 30638a0 Compare July 11, 2025 17:23
@rgrinberg rgrinberg requested a review from Alizter July 11, 2025 17:23
@rgrinberg rgrinberg force-pushed the ps/rr/fix_cram___do_not_re_run_tests_unnecessarily branch 2 times, most recently from d6e20ad to aab71ee Compare July 11, 2025 17:27
Signed-off-by: Rudi Grinberg <[email protected]>

<!-- ps-id: 888f9f4f-14f7-40f0-9ab3-21312e50f0e5 -->

Signed-off-by: Rudi Grinberg <[email protected]>
@rgrinberg rgrinberg force-pushed the ps/rr/fix_cram___do_not_re_run_tests_unnecessarily branch from aab71ee to f0595d7 Compare July 11, 2025 18:00
@rgrinberg rgrinberg merged commit c3992de into main Jul 11, 2025
25 checks passed
@rgrinberg rgrinberg deleted the ps/rr/fix_cram___do_not_re_run_tests_unnecessarily branch July 11, 2025 18:15
Sudha247 pushed a commit to Sudha247/dune that referenced this pull request Jul 23, 2025
maiste added a commit to maiste/opam-repository that referenced this pull request Jul 28, 2025
CHANGES:

### Fixed

- Stop re-running cram tests after promotion when it's not necessary (ocaml/dune#11994,
  @rgrinberg)

- fix: `$ dune subst` should not fail when adding the version field in opam
  files (ocaml/dune#11801, fixes ocaml/dune#11045, @btjorge)

- Kill all processes in the process group after the main process has
  terminated; in particular this avoids background processes in cram tests to
  stick around after the test finished (ocaml/dune#11841, fixes ocaml/dune#11820, @Alizter,
  @Leonidas-from-XIV)

### Added

- `(tests)` stanzas now generate aliases with the test name. To run
  `(test (name a))` you can do `dune build @runtest-a`. (ocaml/dune#11558, grants part of ocaml/dune#10239,
  @Alizter)

- Inline test libraries now produce aliases `runtest-name_of_lib`
  allowing users to run specific inline tests as `dune build
  @runtest-name_of_lib`. (ocaml/dune#11109, partially fixes ocaml/dune#10239, @Alizter)

- feature: `$ dune subst` use version from `dune-project` when no version
  control repository has been detected (ocaml/dune#11801, @btjorge)

- Allow `dune exec` to run concurrently with another instance of dune in watch
  mode (ocaml/dune#11840, @gridbugs)

- Introduce `%{os}`, `%{os_version}`, `%{os_distribution}`, and `%{os_family}`
  percent forms. These have the same values as their opam counterparts.
  (ocaml/dune#11863, @rgrinberg)

- Introduce option `(implicit_transitive_deps false-if-hidden-includes-supported)`
  that is equivalent to `(implicit_transitive_deps false)` when `-H` is
  supported by the compiler (OCaml >= 5.2) and equivalent to
  `(implicit_transitive_deps true)` otherwise. (ocaml/dune#11866, fixes ocaml/dune#11212, @nojb)

- Add `dune describe location` for printing the path to the executable that
  would be run (ocaml/dune#11905, @gridbugs)

- `dune runtest` can now understand absolute paths as well as run tests in
  specific build contexts (ocaml/dune#11936, @Alizter).

- Added 'empty' alias which contains no targets. (ocaml/dune#11556 ocaml/dune#11952 ocaml/dune#11955 ocaml/dune#11956,
  grants ocaml/dune#4161, @Alizter and @rgrinberg)

- Allow `dune promote` to properly run while a watch mode server is running
  (ocaml/dune#12010, @ElectreAAS)

- Add `--alias` and `--alias-rec` flags as an alternative to the `@` and `@@`
  syntax in the command line (ocaml/dune#12043, fixes ocaml/dune#5775, @rgrinberg)

### Changed

- Format long lists in s-expressions to fill the line instead of
  formatting them in a vertical way (ocaml/dune#10892, fixes ocaml/dune#10860, @nojb)

- Switch from MD5 to BLAKE3 for digesting targets and rules. BLAKE3 is both more
  performant and difficult to break than MD5 (ocaml/dune#11735, @rgrinberg, @Alizter)

- Print a warning when `dune build` runs over RPC (ocaml/dune#11836, @gridbugs)

- Stop emitting empty module group wrapper `.js` file in `melange.emit`
  (ocaml/dune#11987, fixes ocaml/dune#11986, @anmonteiro)
maiste added a commit to maiste/opam-repository that referenced this pull request Jul 30, 2025
CHANGES:

### Fixed

- Stop re-running cram tests after promotion when it's not necessary (ocaml/dune#11994,
  @rgrinberg)

- fix: `$ dune subst` should not fail when adding the version field in opam
  files (ocaml/dune#11801, fixes ocaml/dune#11045, @btjorge)

- Kill all processes in the process group after the main process has
  terminated; in particular this avoids background processes in cram tests to
  stick around after the test finished (ocaml/dune#11841, fixes ocaml/dune#11820, @Alizter,
  @Leonidas-from-XIV)

### Added

- `(tests)` stanzas now generate aliases with the test name. To run
  `(test (name a))` you can do `dune build @runtest-a`. (ocaml/dune#11558, grants part of ocaml/dune#10239,
  @Alizter)

- Inline test libraries now produce aliases `runtest-name_of_lib`
  allowing users to run specific inline tests as `dune build
  @runtest-name_of_lib`. (ocaml/dune#11109, partially fixes ocaml/dune#10239, @Alizter)

- feature: `$ dune subst` use version from `dune-project` when no version
  control repository has been detected (ocaml/dune#11801, @btjorge)

- Allow `dune exec` to run concurrently with another instance of dune in watch
  mode (ocaml/dune#11840, @gridbugs)

- Introduce `%{os}`, `%{os_version}`, `%{os_distribution}`, and `%{os_family}`
  percent forms. These have the same values as their opam counterparts.
  (ocaml/dune#11863, @rgrinberg)

- Introduce option `(implicit_transitive_deps false-if-hidden-includes-supported)`
  that is equivalent to `(implicit_transitive_deps false)` when `-H` is
  supported by the compiler (OCaml >= 5.2) and equivalent to
  `(implicit_transitive_deps true)` otherwise. (ocaml/dune#11866, fixes ocaml/dune#11212, @nojb)

- Add `dune describe location` for printing the path to the executable that
  would be run (ocaml/dune#11905, @gridbugs)

- `dune runtest` can now understand absolute paths as well as run tests in
  specific build contexts (ocaml/dune#11936, @Alizter).

- Added 'empty' alias which contains no targets. (ocaml/dune#11556 ocaml/dune#11952 ocaml/dune#11955 ocaml/dune#11956,
  grants ocaml/dune#4161, @Alizter and @rgrinberg)

- Allow `dune promote` to properly run while a watch mode server is running
  (ocaml/dune#12010, @ElectreAAS)

- Add `--alias` and `--alias-rec` flags as an alternative to the `@@` and `@`
  syntax in the command line (ocaml/dune#12043, fixes ocaml/dune#5775, @rgrinberg)

- Added a `(timeout <float>)` field to the `(cram)` stanza to specify per-test
  time limits. Tests exceeding the timeout are terminated with an error.
  (ocaml/dune#12041, @Alizter)

### Changed

- Format long lists in s-expressions to fill the line instead of
  formatting them in a vertical way (ocaml/dune#10892, fixes ocaml/dune#10860, @nojb)

- Switch from MD5 to BLAKE3 for digesting targets and rules. BLAKE3 is both more
  performant and difficult to break than MD5 (ocaml/dune#11735, @rgrinberg, @Alizter)

- Print a warning when `dune build` runs over RPC (ocaml/dune#11833, @gridbugs)

- Stop emitting empty module group wrapper `.js` file in `melange.emit`
  (ocaml/dune#11987, fixes ocaml/dune#11986, @anmonteiro)
maiste added a commit to maiste/opam-repository that referenced this pull request Aug 5, 2025
CHANGES:

### Fixed

- Stop re-running cram tests after promotion when it's not necessary (ocaml/dune#11994,
  @rgrinberg)

- fix: `$ dune subst` should not fail when adding the version field in opam
  files (ocaml/dune#11801, fixes ocaml/dune#11045, @btjorge)

- Kill all processes in the process group after the main process has
  terminated; in particular this avoids background processes in cram tests to
  stick around after the test finished (ocaml/dune#11841, fixes ocaml/dune#11820, @Alizter,
  @Leonidas-from-XIV)

### Added

- `(tests)` stanzas now generate aliases with the test name. To run
  `(test (name a))` you can do `dune build @runtest-a`. (ocaml/dune#11558, grants part of ocaml/dune#10239,
  @Alizter)

- Inline test libraries now produce aliases `runtest-name_of_lib`
  allowing users to run specific inline tests as `dune build
  @runtest-name_of_lib`. (ocaml/dune#11109, partially fixes ocaml/dune#10239, @Alizter)

- feature: `$ dune subst` use version from `dune-project` when no version
  control repository has been detected (ocaml/dune#11801, @btjorge)

- Allow `dune exec` to run concurrently with another instance of dune in watch
  mode (ocaml/dune#11840, @gridbugs)

- Introduce `%{os}`, `%{os_version}`, `%{os_distribution}`, and `%{os_family}`
  percent forms. These have the same values as their opam counterparts.
  (ocaml/dune#11863, @rgrinberg)

- Introduce option `(implicit_transitive_deps false-if-hidden-includes-supported)`
  that is equivalent to `(implicit_transitive_deps false)` when `-H` is
  supported by the compiler (OCaml >= 5.2) and equivalent to
  `(implicit_transitive_deps true)` otherwise. (ocaml/dune#11866, fixes ocaml/dune#11212, @nojb)

- Add `dune describe location` for printing the path to the executable that
  would be run (ocaml/dune#11905, @gridbugs)

- `dune runtest` can now understand absolute paths as well as run tests in
  specific build contexts (ocaml/dune#11936, @Alizter).

- Added 'empty' alias which contains no targets. (ocaml/dune#11556 ocaml/dune#11952 ocaml/dune#11955 ocaml/dune#11956,
  grants ocaml/dune#4161, @Alizter and @rgrinberg)

- Allow `dune promote` to properly run while a watch mode server is running
  (ocaml/dune#12010, @ElectreAAS)

- Add `--alias` and `--alias-rec` flags as an alternative to the `@@` and `@`
  syntax in the command line (ocaml/dune#12043, fixes ocaml/dune#5775, @rgrinberg)

- Added a `(timeout <float>)` field to the `(cram)` stanza to specify per-test
  time limits. Tests exceeding the timeout are terminated with an error.
  (ocaml/dune#12041, @Alizter)

### Changed

- Format long lists in s-expressions to fill the line instead of
  formatting them in a vertical way (ocaml/dune#10892, fixes ocaml/dune#10860, @nojb)

- Switch from MD5 to BLAKE3 for digesting targets and rules. BLAKE3 is both more
  performant and difficult to break than MD5 (ocaml/dune#11735, @rgrinberg, @Alizter)

- Print a warning when `dune build` runs over RPC (ocaml/dune#11833, @gridbugs)

- Stop emitting empty module group wrapper `.js` file in `melange.emit`
  (ocaml/dune#11987, fixes ocaml/dune#11986, @anmonteiro)
maiste added a commit to maiste/opam-repository that referenced this pull request Aug 6, 2025
CHANGES:

### Fixed

- Stop re-running cram tests after promotion when it's not necessary (ocaml/dune#11994,
  @rgrinberg)

- fix: `$ dune subst` should not fail when adding the version field in opam
  files (ocaml/dune#11801, fixes ocaml/dune#11045, @btjorge)

- Kill all processes in the process group after the main process has
  terminated; in particular this avoids background processes in cram tests to
  stick around after the test finished (ocaml/dune#11841, fixes ocaml/dune#11820, @Alizter,
  @Leonidas-from-XIV)

### Added

- `(tests)` stanzas now generate aliases with the test name. To run
  `(test (name a))` you can do `dune build @runtest-a`. (ocaml/dune#11558, grants part of ocaml/dune#10239,
  @Alizter)

- Inline test libraries now produce aliases `runtest-name_of_lib`
  allowing users to run specific inline tests as `dune build
  @runtest-name_of_lib`. (ocaml/dune#11109, partially fixes ocaml/dune#10239, @Alizter)

- feature: `$ dune subst` use version from `dune-project` when no version
  control repository has been detected (ocaml/dune#11801, @btjorge)

- Allow `dune exec` to run concurrently with another instance of dune in watch
  mode (ocaml/dune#11840, @gridbugs)

- Introduce `%{os}`, `%{os_version}`, `%{os_distribution}`, and `%{os_family}`
  percent forms. These have the same values as their opam counterparts.
  (ocaml/dune#11863, @rgrinberg)

- Introduce option `(implicit_transitive_deps false-if-hidden-includes-supported)`
  that is equivalent to `(implicit_transitive_deps false)` when `-H` is
  supported by the compiler (OCaml >= 5.2) and equivalent to
  `(implicit_transitive_deps true)` otherwise. (ocaml/dune#11866, fixes ocaml/dune#11212, @nojb)

- Add `dune describe location` for printing the path to the executable that
  would be run (ocaml/dune#11905, @gridbugs)

- `dune runtest` can now understand absolute paths as well as run tests in
  specific build contexts (ocaml/dune#11936, @Alizter).

- Added 'empty' alias which contains no targets. (ocaml/dune#11556 ocaml/dune#11952 ocaml/dune#11955 ocaml/dune#11956,
  grants ocaml/dune#4161, @Alizter and @rgrinberg)

- Allow `dune promote` to properly run while a watch mode server is running
  (ocaml/dune#12010, @ElectreAAS)

- Add `--alias` and `--alias-rec` flags as an alternative to the `@@` and `@`
  syntax in the command line (ocaml/dune#12043, fixes ocaml/dune#5775, @rgrinberg)

- Added a `(timeout <float>)` field to the `(cram)` stanza to specify per-test
  time limits. Tests exceeding the timeout are terminated with an error.
  (ocaml/dune#12041, @Alizter)

### Changed

- Format long lists in s-expressions to fill the line instead of
  formatting them in a vertical way (ocaml/dune#10892, fixes ocaml/dune#10860, @nojb)

- Switch from MD5 to BLAKE3 for digesting targets and rules. BLAKE3 is both more
  performant and difficult to break than MD5 (ocaml/dune#11735, @rgrinberg, @Alizter)

- Print a warning when `dune build` runs over RPC (ocaml/dune#11833, @gridbugs)

- Stop emitting empty module group wrapper `.js` file in `melange.emit`
  (ocaml/dune#11987, fixes ocaml/dune#11986, @anmonteiro)
maiste added a commit to maiste/opam-repository that referenced this pull request Aug 12, 2025
CHANGES:

### Fixed

- Stop re-running cram tests after promotion when it's not necessary (ocaml/dune#11994,
  @rgrinberg)

- fix: `$ dune subst` should not fail when adding the version field in opam
  files (ocaml/dune#11801, fixes ocaml/dune#11045, @btjorge)

- Kill all processes in the process group after the main process has
  terminated; in particular this avoids background processes in cram tests to
  stick around after the test finished (ocaml/dune#11841, fixes ocaml/dune#11820, @Alizter,
  @Leonidas-from-XIV)

### Added

- `(tests)` stanzas now generate aliases with the test name. To run
  `(test (name a))` you can do `dune build @runtest-a`. (ocaml/dune#11558, grants part of ocaml/dune#10239,
  @Alizter)

- Inline test libraries now produce aliases `runtest-name_of_lib`
  allowing users to run specific inline tests as `dune build
  @runtest-name_of_lib`. (ocaml/dune#11109, partially fixes ocaml/dune#10239, @Alizter)

- feature: `$ dune subst` use version from `dune-project` when no version
  control repository has been detected (ocaml/dune#11801, @btjorge)

- Allow `dune exec` to run concurrently with another instance of dune in watch
  mode (ocaml/dune#11840, @gridbugs)

- Introduce `%{os}`, `%{os_version}`, `%{os_distribution}`, and `%{os_family}`
  percent forms. These have the same values as their opam counterparts.
  (ocaml/dune#11863, @rgrinberg)

- Introduce option `(implicit_transitive_deps false-if-hidden-includes-supported)`
  that is equivalent to `(implicit_transitive_deps false)` when `-H` is
  supported by the compiler (OCaml >= 5.2) and equivalent to
  `(implicit_transitive_deps true)` otherwise. (ocaml/dune#11866, fixes ocaml/dune#11212, @nojb)

- Add `dune describe location` for printing the path to the executable that
  would be run (ocaml/dune#11905, @gridbugs)

- `dune runtest` can now understand absolute paths as well as run tests in
  specific build contexts (ocaml/dune#11936, @Alizter).

- Added 'empty' alias which contains no targets. (ocaml/dune#11556 ocaml/dune#11952 ocaml/dune#11955 ocaml/dune#11956,
  grants ocaml/dune#4161, @Alizter and @rgrinberg)

- Allow `dune promote` to properly run while a watch mode server is running
  (ocaml/dune#12010, @ElectreAAS)

- Add `--alias` and `--alias-rec` flags as an alternative to the `@@` and `@`
  syntax in the command line (ocaml/dune#12043, fixes ocaml/dune#5775, @rgrinberg)

- Added a `(timeout <float>)` field to the `(cram)` stanza to specify per-test
  time limits. Tests exceeding the timeout are terminated with an error.
  (ocaml/dune#12041, @Alizter)

### Changed

- Format long lists in s-expressions to fill the line instead of
  formatting them in a vertical way (ocaml/dune#10892, fixes ocaml/dune#10860, @nojb)

- Switch from MD5 to BLAKE3 for digesting targets and rules. BLAKE3 is both more
  performant and difficult to break than MD5 (ocaml/dune#11735, @rgrinberg, @Alizter)

- Print a warning when `dune build` runs over RPC (ocaml/dune#11833, @gridbugs)

- Stop emitting empty module group wrapper `.js` file in `melange.emit`
  (ocaml/dune#11987, fixes ocaml/dune#11986, @anmonteiro)
maiste added a commit to maiste/opam-repository that referenced this pull request Aug 19, 2025
CHANGES:

### Fixed

- Stop re-running cram tests after promotion when it's not necessary (ocaml/dune#11994,
  @rgrinberg)

- fix: `$ dune subst` should not fail when adding the version field in opam
  files (ocaml/dune#11801, fixes ocaml/dune#11045, @btjorge)

- Kill all processes in the process group after the main process has
  terminated; in particular this avoids background processes in cram tests to
  stick around after the test finished (ocaml/dune#11841, fixes ocaml/dune#11820, @Alizter,
  @Leonidas-from-XIV)

### Added

- `(tests)` stanzas now generate aliases with the test name. To run
  `(test (name a))` you can do `dune build @runtest-a`. (ocaml/dune#11558, grants part of ocaml/dune#10239,
  @Alizter)

- Inline test libraries now produce aliases `runtest-name_of_lib`
  allowing users to run specific inline tests as `dune build
  @runtest-name_of_lib`. (ocaml/dune#11109, partially fixes ocaml/dune#10239, @Alizter)

- feature: `$ dune subst` use version from `dune-project` when no version
  control repository has been detected (ocaml/dune#11801, @btjorge)

- Allow `dune exec` to run concurrently with another instance of dune in watch
  mode (ocaml/dune#11840, @gridbugs)

- Introduce `%{os}`, `%{os_version}`, `%{os_distribution}`, and `%{os_family}`
  percent forms. These have the same values as their opam counterparts.
  (ocaml/dune#11863, @rgrinberg)

- Introduce option `(implicit_transitive_deps false-if-hidden-includes-supported)`
  that is equivalent to `(implicit_transitive_deps false)` when `-H` is
  supported by the compiler (OCaml >= 5.2) and equivalent to
  `(implicit_transitive_deps true)` otherwise. (ocaml/dune#11866, fixes ocaml/dune#11212, @nojb)

- Add `dune describe location` for printing the path to the executable that
  would be run (ocaml/dune#11905, @gridbugs)

- `dune runtest` can now understand absolute paths as well as run tests in
  specific build contexts (ocaml/dune#11936, @Alizter).

- Added 'empty' alias which contains no targets. (ocaml/dune#11556 ocaml/dune#11952 ocaml/dune#11955 ocaml/dune#11956,
  grants ocaml/dune#4161, @Alizter and @rgrinberg)

- Allow `dune promote` to properly run while a watch mode server is running
  (ocaml/dune#12010, @ElectreAAS)

- Add `--alias` and `--alias-rec` flags as an alternative to the `@@` and `@`
  syntax in the command line (ocaml/dune#12043, fixes ocaml/dune#5775, @rgrinberg)

- Added a `(timeout <float>)` field to the `(cram)` stanza to specify per-test
  time limits. Tests exceeding the timeout are terminated with an error.
  (ocaml/dune#12041, @Alizter)

### Changed

- Format long lists in s-expressions to fill the line instead of
  formatting them in a vertical way (ocaml/dune#10892, fixes ocaml/dune#10860, @nojb)

- Switch from MD5 to BLAKE3 for digesting targets and rules. BLAKE3 is both more
  performant and difficult to break than MD5 (ocaml/dune#11735, @rgrinberg, @Alizter)

- Print a warning when `dune build` runs over RPC (ocaml/dune#11833, @gridbugs)

- Stop emitting empty module group wrapper `.js` file in `melange.emit`
  (ocaml/dune#11987, fixes ocaml/dune#11986, @anmonteiro)
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