From 68834b7841e37a902f1e4bdd3577a4a825e62e2f Mon Sep 17 00:00:00 2001 From: blokyk Date: Mon, 27 Apr 2026 23:14:16 +0200 Subject: [PATCH] tests/f1/repl-characterization: match golden text correctly it seems like the repl_characterization tests have not been actually running correctly for a while, since tests from more than 2 years ago that should have failed have been in the test suite and OK'd thus far. the reason why some tests passed when they shouldn't have is because the AST nodes used by the characterization parser didn't have a correct comparison operator, so the tests would only fail when one of the output line was of the wrong *kind*, but not if it had the wrong *value*. this was fixed by simply adding a default comparator to the TextNode type as well as every derived class. i've taken the occasion to fix the tests that were failing now that we are actually being strict about them. most of these are pretty boring changes like indentation, but if possible i'd like to get an explicit ACK from at least one or two people on the core team that these changes are okay. for reference, here is the zulip thread that lead to this CL: https://zulip.lix.systems/#narrow/channel/8-Testing-.2F-functional2/topic/.E2.9C.94.20repl-characterization.20tests.20not.20failing/near/10276 ps: i had to turn clang-format off around the macro for text node declarations because it re-formatted the entire macro, causing a lot of noising and producing uglier code; if this isn't okay or if there's a better alternative, please let me know! Change-Id: I1d6d92eb3e9cb705ecfeccd7b9294d6cfd7fe25b --- lix/libcmd/repl.cc | 2 +- .../data/basic_repl.test | 24 ++++----- .../data/builtins_warn.test | 11 ++-- .../data/debug_frames.test | 50 +++++++++--------- .../repl_characterization/data/errors.test | 2 +- .../data/no_nested_debuggers.test | 4 +- .../data/regression_9917.test | 52 +++++++++---------- .../data/regression_l592.test | 2 +- ..._destructure_without_dotdotdot_errors.test | 2 +- .../data/repl_overlays_error.test | 2 +- .../data/repl_printing.test | 10 ++-- .../data/stacktrace_oob.test | 2 +- .../data/stacktrace_relative.test | 6 +-- .../tests/cli-literate-parser.hh | 6 ++- 14 files changed, 90 insertions(+), 85 deletions(-) diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index f95bc7595..3a006a3c3 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -817,7 +817,7 @@ void NixRepl::initDebugBuiltinCommands() {.aliases = {"st"}, .debugModeOnly = true, .help = "Show current trace. If an integer is provided, this switches to that stack " - "beforehand. If the integer has an explicit + or - sign, it is treated as" + "beforehand. If the integer has an explicit + or - sign, it is treated as " "relative to the current stack index.", .section = "Debug mode", .positionalArgsSpecifiers = {{.placeholderText = "integer index", .optional = true}}} diff --git a/tests/functional/repl_characterization/data/basic_repl.test b/tests/functional/repl_characterization/data/basic_repl.test index 51cdb6fec..5c7245cbb 100644 --- a/tests/functional/repl_characterization/data/basic_repl.test +++ b/tests/functional/repl_characterization/data/basic_repl.test @@ -14,9 +14,9 @@ Expect the trace to not contain any traceback: nix-repl> f 2 error: … while evaluating a path segment - at «string»:1:10: - 1| a: "" + a - | ^ + at «string»:1:13: + 1| f = a: "" + a + | ^ error: cannot coerce an integer to a string: 2 @@ -33,14 +33,14 @@ Expect the trace to have traceback: | ^ … while calling anonymous lambda - at «string»:1:2: - 1| a: "" + a - | ^ + at «string»:1:5: + 1| f = a: "" + a + | ^ … while evaluating a path segment - at «string»:1:10: - 1| a: "" + a - | ^ + at «string»:1:13: + 1| f = a: "" + a + | ^ error: cannot coerce an integer to a string: 2 @@ -52,8 +52,8 @@ Turning it off should also work: nix-repl> f 2 error: … while evaluating a path segment - at «string»:1:10: - 1| a: "" + a - | ^ + at «string»:1:13: + 1| f = a: "" + a + | ^ error: cannot coerce an integer to a string: 2 diff --git a/tests/functional/repl_characterization/data/builtins_warn.test b/tests/functional/repl_characterization/data/builtins_warn.test index 9955983f2..2596f975d 100644 --- a/tests/functional/repl_characterization/data/builtins_warn.test +++ b/tests/functional/repl_characterization/data/builtins_warn.test @@ -1,12 +1,13 @@ @args --debugger --option debugger-on-warn true Enter debugger with debugger-on-warn set. - nix-repl> let inspect = v: builtins.warn "inspect is deprecated" (throw "this happens after"); in inspect { } + warning: inspect is deprecated + warning: builtins.warn reached - warning: inspect is deprecated! - warning: builtins.warn reached! +Continuing after the warn then breaks on the `throw` + nix-repl> :c + error: this happens after +Finally, continuing after that error exits and prints the error nix-repl> :c - - { } error: this happens after diff --git a/tests/functional/repl_characterization/data/debug_frames.test b/tests/functional/repl_characterization/data/debug_frames.test index ad3198f9a..b97a76f56 100644 --- a/tests/functional/repl_characterization/data/debug_frames.test +++ b/tests/functional/repl_characterization/data/debug_frames.test @@ -14,37 +14,37 @@ nix-repl> :? The following commands are available: - Evaluate and print expression - = Bind expression to variable - :?, :help Print help about all commands (this content) - :a, :add Add attributes from resulting set to scope - :b, :build Build a derivation - :bl, :build-with-gc-roots Build a derivation, creating GC roots in the working directory - :doc Show documentation for the provided function (experimental lambda support) - :e, :edit Open package or function in $EDITOR - :env Show environment stack - :i, :build-and-install Build derivation, then install result into current profile - :l, :load Load Nix expression and add it to scope - :log Show logs for a derivation - :p, :print Evaluate and print expression recursively - Strings are printed directly, without escaping. - :q, :quit Exit the REPL - :r, :reload Reload all files successfully loaded - :sh, :shell Build dependencies of derivation, then start nix-shell - :t, :type Describe result of evaluation - :te, :trace-enable [bool] Enable, disable, or toggle showing traces for errors - :u, :use Build derivation, then start nix-shell + Evaluate and print expression + = Bind expression to variable + :?, :help Print help about all commands (this content) + :a, :add Add attributes from resulting set to scope + :b, :build Build a derivation + :bl, :build-with-gc-roots Build a derivation, creating GC roots in the working directory + :doc Show documentation for the provided function (experimental lambda support) + :e, :edit Open package or function in $EDITOR + :env Show environment stack + :i, :build-and-install Build derivation, then install result into current profile + :l, :load Load Nix expression and add it to scope + :log Show logs for a derivation + :p, :print Evaluate and print expression recursively + Strings are printed directly, without escaping. + :q, :quit Exit the REPL + :r, :reload Reload all files successfully loaded + :sh, :shell Build dependencies of derivation, then start nix-shell + :t, :type Describe result of evaluation + :te, :trace-enable [bool] Enable, disable, or toggle showing traces for errors + :u, :use Build derivation, then start nix-shell Debug mode commands - :bt, :backtrace Show trace stack - :c, :continue Go until end of program, exception or builtins.break - :s, :step Go one step - :st, :show-trace [integer index] Show current trace. If an integer is provided, this switches to that stack beforehand. If the integer has an explicit + or - sign, it is treated as relative to the current stack index. + :bt, :backtrace Show trace stack + :c, :continue Go until end of program, exception or builtins.break + :s, :step Go one step + :st, :show-trace [integer index] Show current trace. If an integer is provided, this switches to that stack beforehand. If the integer has an explicit + or - sign, it is treated as relative to the current stack index. Flakes commands - :lf, :load-flake Load Nix flake and add it to the scope + :lf, :load-flake Load Nix flake and add it to the scope we can now inspect state nix-repl> :bt diff --git a/tests/functional/repl_characterization/data/errors.test b/tests/functional/repl_characterization/data/errors.test index 47d7e7e13..f6e2075f2 100644 --- a/tests/functional/repl_characterization/data/errors.test +++ b/tests/functional/repl_characterization/data/errors.test @@ -2,7 +2,7 @@ Errors at the top of an expression are printed normally: nix-repl> builtins.throw "Evil puppy detected!!!" error: - … while calling the 'throw' builtin + … caused by explicit throw at «string»:1:1: 1| builtins.throw "Evil puppy detected!!!" | ^ diff --git a/tests/functional/repl_characterization/data/no_nested_debuggers.test b/tests/functional/repl_characterization/data/no_nested_debuggers.test index 0199407f6..50311eddd 100644 --- a/tests/functional/repl_characterization/data/no_nested_debuggers.test +++ b/tests/functional/repl_characterization/data/no_nested_debuggers.test @@ -14,7 +14,7 @@ print the error, skip the breakpoint, etc as appropriate. nix-repl> builtins.throw "foo" error: - … while calling the 'throw' builtin + … caused by explicit throw at «string»:1:1: 1| builtins.throw "foo" | ^ @@ -22,7 +22,7 @@ print the error, skip the breakpoint, etc as appropriate. error: foo nix-repl> assert false; 2 - error: assertion 'false' failed + error: assertion failed at «string»:1:1: 1| assert false; 2 | ^ diff --git a/tests/functional/repl_characterization/data/regression_9917.test b/tests/functional/repl_characterization/data/regression_9917.test index 451c4efec..ec147abd4 100644 --- a/tests/functional/repl_characterization/data/regression_9917.test +++ b/tests/functional/repl_characterization/data/regression_9917.test @@ -14,56 +14,56 @@ This test ensures that continues don't skip opportunities to enter the debugger. 0: error: breakpoint reached $TEST_DATA/regression_9917.nix:3:5 - 2| a = builtins.trace "before inner break" ( - 3| builtins.break { msg = "hello"; } - | ^ - 4| ); + 2| a = builtins.trace "before inner break" ( + 3| builtins.break { msg = "hello"; } + | ^ + 4| ); 1: while calling a function $TEST_DATA/regression_9917.nix:3:5 - 2| a = builtins.trace "before inner break" ( - 3| builtins.break { msg = "hello"; } - | ^ - 4| ); + 2| a = builtins.trace "before inner break" ( + 3| builtins.break { msg = "hello"; } + | ^ + 4| ); 2: while calling a function $TEST_DATA/regression_9917.nix:2:7 - 1| let - 2| a = builtins.trace "before inner break" ( - | ^ - 3| builtins.break { msg = "hello"; } + 1| let + 2| a = builtins.trace "before inner break" ( + | ^ + 3| builtins.break { msg = "hello"; } 3: while calling a function $TEST_DATA/regression_9917.nix:6:5 - 5| b = builtins.trace "before outer break" ( - 6| builtins.break a - | ^ - 7| ); + 5| b = builtins.trace "before outer break" ( + 6| builtins.break a + | ^ + 7| ); 4: while calling a function $TEST_DATA/regression_9917.nix:5:7 - 4| ); - 5| b = builtins.trace "before outer break" ( - | ^ - 6| builtins.break a + 4| ); + 5| b = builtins.trace "before outer break" ( + | ^ + 6| builtins.break a 5: while evaluating a 'let' expression $TEST_DATA/regression_9917.nix:1:1 - 1| let - | ^ - 2| a = builtins.trace "before inner break" ( + 1| let + | ^ + 2| a = builtins.trace "before inner break" ( 6: while evaluating the file '$TEST_DATA/regression_9917.nix': $TEST_DATA/regression_9917.nix:1:1 - 1| let - | ^ - 2| a = builtins.trace "before inner break" ( + 1| let + | ^ + 2| a = builtins.trace "before inner break" ( nix-repl> :c diff --git a/tests/functional/repl_characterization/data/regression_l592.test b/tests/functional/repl_characterization/data/regression_l592.test index b0ba331ae..ded026b60 100644 --- a/tests/functional/repl_characterization/data/regression_l592.test +++ b/tests/functional/repl_characterization/data/regression_l592.test @@ -1,6 +1,6 @@ @args --debugger - breakpoint reached + info: breakpoint reached nix-repl> :quit error: diff --git a/tests/functional/repl_characterization/data/repl_overlays_destructure_without_dotdotdot_errors.test b/tests/functional/repl_characterization/data/repl_overlays_destructure_without_dotdotdot_errors.test index daf1f27bd..ca5ccc01f 100644 --- a/tests/functional/repl_characterization/data/repl_overlays_destructure_without_dotdotdot_errors.test +++ b/tests/functional/repl_characterization/data/repl_overlays_destructure_without_dotdotdot_errors.test @@ -7,4 +7,4 @@ 3| in 4| {currentSystem}: final: prev: { | ^ - 5| inherit puppy;\n + 5| inherit puppy; diff --git a/tests/functional/repl_characterization/data/repl_overlays_error.test b/tests/functional/repl_characterization/data/repl_overlays_error.test index 086136b62..a28708091 100644 --- a/tests/functional/repl_characterization/data/repl_overlays_error.test +++ b/tests/functional/repl_characterization/data/repl_overlays_error.test @@ -3,7 +3,7 @@ @args ${PWD}/extra_data/repl-overlay-fail.nix @should-start false error: - … while evaluating variable final + … while evaluating final at «string»:6:1: 5| in 6| final diff --git a/tests/functional/repl_characterization/data/repl_printing.test b/tests/functional/repl_characterization/data/repl_printing.test index 72b87e9c4..f41772ec1 100644 --- a/tests/functional/repl_characterization/data/repl_printing.test +++ b/tests/functional/repl_characterization/data/repl_printing.test @@ -106,9 +106,9 @@ Printing a derivation or something nix-repl> fakeDrv = let drvAttrs = { builder = "meow"; system = "meower"; name = "mrowmrow"; }; in { inherit (drvAttrs) builder system name; inherit drvAttrs; type = "derivation"; } nix-repl> :p fakeDrv { - builder = "meow"; - drvAttrs = «3 attributes elided»; - name = "mrowmrow"; - system = "meower"; - type = "derivation"; + builder = "meow"; + drvAttrs = «3 attributes elided»; + name = "mrowmrow"; + system = "meower"; + type = "derivation"; } diff --git a/tests/functional/repl_characterization/data/stacktrace_oob.test b/tests/functional/repl_characterization/data/stacktrace_oob.test index 8bafd99fd..a50f3489e 100644 --- a/tests/functional/repl_characterization/data/stacktrace_oob.test +++ b/tests/functional/repl_characterization/data/stacktrace_oob.test @@ -42,7 +42,7 @@ frames from 0 up to 4 work fine absolute frames out of bounds print an error nix-repl> :st 5 - error: stack index must be between 0 and 4, but was 5 + error: stack index must be between 0 and 4 (inclusive), but was 5 argument-less :st is still at the same after absolute oob nix-repl> :st diff --git a/tests/functional/repl_characterization/data/stacktrace_relative.test b/tests/functional/repl_characterization/data/stacktrace_relative.test index 99539f94d..2728a18ca 100644 --- a/tests/functional/repl_characterization/data/stacktrace_relative.test +++ b/tests/functional/repl_characterization/data/stacktrace_relative.test @@ -8,7 +8,7 @@ absolute indices still work: 1: while calling a function «string»:1:12 - 1| let f = _: throw "x_x"; x = f 5; in x + 1| let f = _: throw "x_x"; x = f 5; in x | ^ Env level 0 @@ -29,7 +29,7 @@ index with + goes up the stack relative to current (1 in this case): 4: while evaluating a 'let' expression «string»:1:1 - 1| let f = _: throw "x_x"; x = f 5; in x + 1| let f = _: throw "x_x"; x = f 5; in x | ^ Env level 0 @@ -47,7 +47,7 @@ index with - goes down and is also relative to current (4): 3: while calling a function «string»:1:29 - 1| let f = _: throw "x_x"; x = f 5; in x + 1| let f = _: throw "x_x"; x = f 5; in x | ^ Env level 0 diff --git a/tests/unit/libutil-support/tests/cli-literate-parser.hh b/tests/unit/libutil-support/tests/cli-literate-parser.hh index 2eace1e3d..55ea5eb81 100644 --- a/tests/unit/libutil-support/tests/cli-literate-parser.hh +++ b/tests/unit/libutil-support/tests/cli-literate-parser.hh @@ -26,7 +26,7 @@ struct BaseNode { virtual auto kind() const -> std::string = 0; virtual auto emitNewlineAfter() const -> bool = 0; - auto operator<=>(const BaseNode &rhs) const = default; + auto operator<=>(const BaseNode & rhs) const = default; }; /** @@ -37,10 +37,12 @@ struct TextNode : BaseNode { std::string text; explicit TextNode(std::string text) : text(text) {} + auto operator<=>(const TextNode & rhs) const = default; }; std::ostream &operator<<(std::ostream &output, const TextNode &node); +// clang-format off #define DECLARE_TEXT_NODE(NAME, NEEDS_NEWLINE, SHOULD_COMPARE) \ struct NAME : TextNode { \ using TextNode::TextNode; \ @@ -49,7 +51,9 @@ std::ostream &operator<<(std::ostream &output, const TextNode &node); auto kind() const -> std::string override { return #NAME; } \ auto emitNewlineAfter() const -> bool override { return NEEDS_NEWLINE; } \ auto shouldCompare() const -> bool override { return SHOULD_COMPARE; } \ + auto operator<=>(const NAME & rhs) const = default; \ }; +// clang-format on /* name, needsNewline, shouldCompare */ DECLARE_TEXT_NODE(Prompt, false, false)