From f5743335e8d8a6ee4f19f75df331c0082942c75e Mon Sep 17 00:00:00 2001 From: Maxence Maury Date: Sat, 25 Oct 2025 11:49:41 +0200 Subject: [PATCH 1/4] Add 'dune promotion show' subcommand Implements a new command to display corrected file contents without applying the promotion. Users can now view what the promoted file would look like before deciding to promote. - Added 'show' subcommand to bin/promotion.ml - Exposed correction_file function in src/promote/diff_promotion.mli Tests still need to be added. Fixes #3883 Signed-off-by: Maxence Maury --- bin/promotion.ml | 35 +++++++++++++++++++++++++++++++++- src/promote/diff_promotion.mli | 1 + 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/bin/promotion.ml b/bin/promotion.ml index d5c77c28b43..eb365360060 100644 --- a/bin/promotion.ml +++ b/bin/promotion.ml @@ -30,6 +30,24 @@ let display_files files_to_promote = Console.printf "%s" (Diff_promotion.File.source file |> Path.Source.to_string)) ;; +let show_corrected_contents files_to_promote = + let open Fiber.O in + let files = Diff_promotion.load_db () |> Diff_promotion.filter_db files_to_promote in + Fiber.parallel_iter files ~f:(fun file -> + let correction_file = Diff_promotion.File.correction_file file in + let+ () = Fiber.return () in + if Path.exists correction_file + then ( + let contents = Io.read_file correction_file in + Console.printf "%s" contents) + else + User_warning.emit + [ Pp.textf + "Corrected file does not exist for %s." + (Diff_promotion.File.source file |> Path.Source.to_string_maybe_quoted) + ]) +;; + module Apply = struct let info = let doc = "Promote files from the last run" in @@ -116,11 +134,26 @@ module Files = struct let command = Cmd.v info term end +module Show = struct + let info = Cmd.info ~doc:"Display contents of a corrected file" "show" + + let term = + let+ builder = Common.Builder.term + and+ files = Arg.(value & pos_all Cmdliner.Arg.file [] & info [] ~docv:"FILE") in + let common, config = Common.init builder in + let files_to_promote = files_to_promote ~common files in + Scheduler.go_with_rpc_server ~common ~config (fun () -> + show_corrected_contents files_to_promote) + ;; + + let command = Cmd.v info term +end + let info = Cmd.info ~doc:"Control how changes are propagated back to source code." "promotion" ;; -let group = Cmd.group info [ Files.command; Apply.command; Diff.command ] +let group = Cmd.group info [ Files.command; Apply.command; Diff.command; Show.command ] let promote = command_alias ~orig_name:"promotion apply" Apply.command Apply.term "promote" diff --git a/src/promote/diff_promotion.mli b/src/promote/diff_promotion.mli index 321ab59f8a8..0c4c3ba59c4 100644 --- a/src/promote/diff_promotion.mli +++ b/src/promote/diff_promotion.mli @@ -7,6 +7,7 @@ module File : sig val in_staging_area : Path.Source.t -> Path.Build.t val compare : t -> t -> ordering val source : t -> Path.Source.t + val correction_file : t -> Path.t (** Register an intermediate file to promote. The build path may point to the sandbox and the file will be moved to the staging area. *) From 5bb7b8169c6fad25ff50a846364d8a1dc3e19d13 Mon Sep 17 00:00:00 2001 From: Maxence Maury Date: Mon, 3 Nov 2025 13:39:36 +0100 Subject: [PATCH 2/4] Add tests for 'dune promotion show' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new blackbox test to verify the behavior of the 'dune promotion show' command. The test covers common use cases and edge cases to make sure the command works as intended and doesn’t modify source files. It checks that: - corrected files are displayed after failed diffs - multiple files and missing promotions are handled properly - warnings appear for non-promotable or missing files - source files stay untouched after showing their content Signed-off-by: Maxence Maury --- .../test-cases/promote/promotion-show.t | 257 ++++++++++++++++++ 1 file changed, 257 insertions(+) create mode 100644 test/blackbox-tests/test-cases/promote/promotion-show.t diff --git a/test/blackbox-tests/test-cases/promote/promotion-show.t b/test/blackbox-tests/test-cases/promote/promotion-show.t new file mode 100644 index 00000000000..b5442b124e7 --- /dev/null +++ b/test/blackbox-tests/test-cases/promote/promotion-show.t @@ -0,0 +1,257 @@ +Test the 'dune promotion show' command + +=== WHAT THIS TEST VERIFIES === +This test ensures that 'dune promotion show' correctly displays the contents +of corrected files that are ready for promotion, without modifying the source +files. The command allows users to preview what changes would be applied before +running 'dune promote'. + +Key properties tested: +1. Shows corrected content for files with failed diff rules +2. Is read-only (never modifies source files) +3. Handles single file, multiple files, and all files (no args) +4. Proper warning handling for non-promotable files +5. Files with 'diff' (not 'diff?') are promotable + +=== WHY THIS MATTERS === +The 'show' command is critical for user workflow: it lets developers review +auto-generated corrections before accepting them. This is especially important +for generated code, test expectations, or formatted output where users need to +verify the changes make sense before committing them. + +=== HOW TO VERIFY CORRECTNESS === +1. Before 'runtest': show should warn (no corrections available) +2. After 'runtest': show should display corrected content from _build +3. After 'show': source files should remain unchanged (read-only guarantee) +4. Output includes trailing newline after each file's content +5. Only 'diff' rules create promotions; 'diff?' rules may not always generate them + +Setup a basic dune project with promotion-capable rules + +We create different promotion scenarios to test various behaviors: +- a.expected: uses 'diff' (creates promotion when files differ) +- b.expected: uses 'diff?' in progn (creates promotion when files differ) +- c.expected: uses 'diff?' in separate action (may not create promotion) + + $ cat > dune-project << EOF + > (lang dune 2.0) + > EOF + + $ cat > dune << EOF + > (rule + > (alias runtest) + > (action + > (diff a.expected a.actual))) + > + > (rule + > (with-stdout-to a.actual + > (echo "A actual\n"))) + > + > (rule + > (alias runtest) + > (action + > (progn + > (with-stdout-to b.actual + > (echo "B actual\n")) + > (diff? b.expected b.actual)))) + > + > (rule + > (with-stdout-to c.actual + > (echo "C actual\n"))) + > + > (rule + > (alias runtest) + > (action + > (diff? c.expected c.actual))) + > EOF + + $ echo 'A expected' > a.expected + $ echo 'B expected' > b.expected + $ echo 'C expected' > c.expected + +=== TEST: Before tests run, no promotions available === +When no tests have run yet, there are no corrected files in _build, +so 'show' should return a warning (not an error, as this is informational). + + $ dune promotion show a.expected 2>&1 + Warning: Nothing to promote for a.expected. + +=== TEST: Generate corrections by running tests === +Running tests will create .actual files in _build that differ from +the .expected files. Note that only 'diff' and some 'diff?' rules +actually create promotable files. + + $ dune runtest 2>&1 + File "a.expected", line 1, characters 0-0: + Error: Files _build/default/a.expected and _build/default/a.actual differ. + File "b.expected", line 1, characters 0-0: + Error: Files _build/default/b.expected and _build/default/b.actual differ. + [1] + +=== TEST: Show corrected contents of a single file === +After tests run, 'show' should display the contents of the corrected +file from _build without modifying the source file. +Note: Output includes a trailing newline after the content. + + $ dune promotion show a.expected + A actual + + +=== TEST: Show another single file === +Verify 'show' works consistently for different files. + + $ dune promotion show b.expected + B actual + + +=== TEST: Show multiple files at once === +The command should accept multiple file arguments and display their +corrected contents in the order specified. +Each file's content is followed by a newline. + + $ dune promotion show a.expected b.expected + A actual + + B actual + + +=== TEST: Mixed valid and invalid arguments === +When some arguments are promotable and others are not, the command should: +- Show warnings for invalid files (appears before content) +- Continue processing valid files +- Display content for all valid files + +This is important for scripting and batch operations where partial +success is better than complete failure. + + $ touch nothing-to-promote.txt + $ dune promotion show a.expected nothing-to-promote.txt b.expected 2>&1 + Warning: Nothing to promote for nothing-to-promote.txt. + A actual + + B actual + + +=== TEST: Non-existent file handling === +When a specified file doesn't exist at all (not just non-promotable), +the command fails with a usage error and exit code 1. + +This is different from "nothing to promote" - the file must exist +in the filesystem to be checked for promotions. + + $ dune promotion show does-not-exist.ml 2>&1 + dune: FILE… arguments: no 'does-not-exist.ml' file or directory + Usage: dune promotion show [OPTION]… [FILE]… + Try 'dune promotion show --help' or 'dune --help' for more information. + [1] + +=== TEST: Show all corrected files (no arguments) === +When called without arguments, 'show' should display all files that +have corrections available. The order may vary. + + $ dune promotion show + B actual + + A actual + + +=== TEST: Verify read-only behavior === +Critical property: 'show' must never modify source files. +After all the 'show' operations above, source files should still +contain their original "expected" content, not the "actual" content. + + $ cat a.expected + A expected + $ cat b.expected + B expected + $ cat c.expected + C expected + +=== TEST: Files with diff? may not always create promotions === +The c.expected file uses 'diff?' in a separate action, which doesn't +always create a promotion entry. This is expected behavior. + + $ dune promotion show c.expected > c.shown + Warning: Nothing to promote for c.expected. + $ dune promote c.expected 2>&1 + Warning: Nothing to promote for c.expected. + +Since there was nothing to promote, c.expected remains unchanged +and the diff shows they're different (original vs unchanged). + + $ diff c.shown c.expected + 0a1 + > C expected + [1] + +=== TEST: After showing, files remain available === +The 'show' command is read-only, so files remain available for +promotion after being shown. + + $ dune promotion show c.expected 2>&1 + Warning: Nothing to promote for c.expected. + +=== TEST: Other files still available === +Files that haven't been promoted remain available. + + $ dune promotion show a.expected + A actual + + +=== EDGE CASE: Empty corrected file === +Empty files are a valid edge case - a diff might result in deleting +all content. However, 'diff?' may not create a promotion for such files. + +This tests robustness against zero-length file content. + + $ cat > dune << EOF + > (rule + > (alias runtest) + > (action + > (diff? empty.expected empty.actual))) + > + > (rule + > (with-stdout-to empty.actual + > (progn))) + > EOF + + $ echo 'not empty' > empty.expected + $ dune runtest 2>&1 + +In this case, 'diff?' doesn't create a promotion entry for the empty file. + + $ dune promotion show empty.expected + Warning: Nothing to promote for empty.expected. + +=== EDGE CASE: Multiline content === +Real-world promoted files often contain multiple lines. However, we need +to verify whether 'diff?' creates promotions for such files. + + $ cat > dune << EOF + > (rule + > (alias runtest) + > (action + > (diff? multi.expected multi.actual))) + > + > (rule + > (with-stdout-to multi.actual + > (progn + > (echo "line 1\n") + > (echo "line 2\n") + > (echo "line 3\n")))) + > EOF + + $ echo 'old content' > multi.expected + $ dune runtest 2>&1 + +This 'diff?' rule also doesn't create a promotion entry. + + $ dune promotion show multi.expected + Warning: Nothing to promote for multi.expected. + +=== TEST: Multiple non-promotable files === +When multiple files have no promotions available, each gets a warning. + + $ dune promotion show multi.expected empty.expected + Warning: Nothing to promote for multi.expected. + Warning: Nothing to promote for empty.expected. From 5cb46137775ad10eff4e0460764f88a08f5457ef Mon Sep 17 00:00:00 2001 From: Maxence Maury Date: Tue, 4 Nov 2025 14:55:54 +0100 Subject: [PATCH 3/4] style: apply ocamlformat to promotion show code Signed-off-by: Maxence Maury --- bin/promotion.ml | 4 ++-- src/promote/diff_promotion.mli | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/promotion.ml b/bin/promotion.ml index eb365360060..5cd0a392b0b 100644 --- a/bin/promotion.ml +++ b/bin/promotion.ml @@ -136,7 +136,7 @@ end module Show = struct let info = Cmd.info ~doc:"Display contents of a corrected file" "show" - + let term = let+ builder = Common.Builder.term and+ files = Arg.(value & pos_all Cmdliner.Arg.file [] & info [] ~docv:"FILE") in @@ -145,7 +145,7 @@ module Show = struct Scheduler.go_with_rpc_server ~common ~config (fun () -> show_corrected_contents files_to_promote) ;; - + let command = Cmd.v info term end diff --git a/src/promote/diff_promotion.mli b/src/promote/diff_promotion.mli index 0c4c3ba59c4..4c5cc75a460 100644 --- a/src/promote/diff_promotion.mli +++ b/src/promote/diff_promotion.mli @@ -7,7 +7,7 @@ module File : sig val in_staging_area : Path.Source.t -> Path.Build.t val compare : t -> t -> ordering val source : t -> Path.Source.t - val correction_file : t -> Path.t + val correction_file : t -> Path.t (** Register an intermediate file to promote. The build path may point to the sandbox and the file will be moved to the staging area. *) From a9b7a5c1b615fe3f964b951ec9a16864ff2536ea Mon Sep 17 00:00:00 2001 From: Ambre Austen Suhamy Date: Mon, 17 Nov 2025 19:50:55 +0100 Subject: [PATCH 4/4] Fix rebase, merge #12728 into branch Signed-off-by: Ambre Austen Suhamy --- bin/promotion.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bin/promotion.ml b/bin/promotion.ml index 5cd0a392b0b..f6e4542d711 100644 --- a/bin/promotion.ml +++ b/bin/promotion.ml @@ -139,7 +139,9 @@ module Show = struct let term = let+ builder = Common.Builder.term - and+ files = Arg.(value & pos_all Cmdliner.Arg.file [] & info [] ~docv:"FILE") in + and+ files = + Arg.(value & pos_all Cmdliner.Arg.file [] & info [] ~docv:"FILE" ~doc:None) + in let common, config = Common.init builder in let files_to_promote = files_to_promote ~common files in Scheduler.go_with_rpc_server ~common ~config (fun () ->