fix: address review feedback for promotion show command

- Change User_warning.emit to User_error.raise when no promotion available
- Remove unnecessary Cmdliner.Arg module prefix for 'file' argument
- Remove redundant 2>&1 redirections from cram tests

Signed-off-by: Maxence Maury <[email protected]>

Author: Maxence Maury

bin/promotion.ml

@@ -41,7 +41,7 @@ let show_corrected_contents files_to_promote =
       let contents = Io.read_file correction_file in
       Console.printf "%s" contents)
     else
-      User_warning.emit
+      User_error.raise
         [ Pp.textf
             "Corrected file does not exist for %s."
             (Diff_promotion.File.source file |> Path.Source.to_string_maybe_quoted)
@@ -139,7 +139,7 @@ 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 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 () ->

test/blackbox-tests/test-cases/promote/promotion-show.t

@@ -73,15 +73,15 @@ We create different promotion scenarios to test various behaviors:
 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
+  $ dune promotion show a.expected
   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
+  $ dune runtest
   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:
@@ -125,7 +125,7 @@ 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
+  $ dune promotion show a.expected nothing-to-promote.txt b.expected
   Warning: Nothing to promote for nothing-to-promote.txt.
   A actual
 
@@ -139,7 +139,7 @@ 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 promotion show does-not-exist.ml
   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.
@@ -173,7 +173,7 @@ 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
+  $ dune promote c.expected
   Warning: Nothing to promote for c.expected.
 
 Since there was nothing to promote, c.expected remains unchanged
@@ -188,7 +188,7 @@ and the diff shows they're different (original vs unchanged).
 The 'show' command is read-only, so files remain available for
 promotion after being shown.
 
-  $ dune promotion show c.expected 2>&1
+  $ dune promotion show c.expected
   Warning: Nothing to promote for c.expected.
 
 === TEST: Other files still available ===
@@ -216,7 +216,7 @@ This tests robustness against zero-length file content.
   > EOF
 
   $ echo 'not empty' > empty.expected
-  $ dune runtest 2>&1
+  $ dune runtest
 
 In this case, 'diff?' doesn't create a promotion entry for the empty file.
 
@@ -242,7 +242,7 @@ to verify whether 'diff?' creates promotions for such files.
   > EOF
 
   $ echo 'old content' > multi.expected
-  $ dune runtest 2>&1
+  $ dune runtest
 
 This 'diff?' rule also doesn't create a promotion entry.