Diagnose nondeterministic Symbol comparisons in tests #143

New issue

Open

opened 2024-03-17 00:09:19 +00:00 by jade · 1 comment

jade commented

2024-03-17 00:09:19 +00:00

Owner

We reproduced the below bug in building from a different nixpkgs, where the ordering of the attr sets from the debugger changed. Although the answer to that is "sort the debugger output", we should have a way to ferret out similar sorts of nondeterminism due to Symbol pointer comparisons in a way that will cause it to fail way earlier.

nix>     With diff:
nix>     @@ -11,5 +11,5 @@
nix>      Output ""
nix>      Output "Env level 2"
nix>     -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b
reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation"
nix>     +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf
 dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation"
nix>      Command "meow"
nix>      Output "2"
nix>     @@ -27,5 +27,5 @@
nix>      Output ""
nix>      Output "Env level 1"
nix>     -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b
reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation"
nix>     +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf
 dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation"
nix>      Command ":c"
nix>      Output "trace: before inner break"
nix>     @@ -41,5 +41,5 @@
nix>      Output ""
nix>      Output "Env level 2"
nix>     -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b
reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation"
nix>     +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf
 dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation"
nix>      Command "meow'"
nix>      Output "3"
nix>     @@ -57,3 +57,3 @@
nix>      Output ""
nix>      Output "Env level 1"
nix>     -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b
reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation"\n
nix>     +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf
 dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation"\n
nix>     [  FAILED  ] ReplSessionTest.stack_vars (32 ms)
nix>     [----------] 5 tests from ReplSessionTest (118 ms total)
nix>     [----------] Global test environment tear-down
nix>     [==========] 5 tests from 1 test suite ran. (118 ms total)
nix>     [  PASSED  ] 3 tests.
nix>     [  FAILED  ] 2 tests, listed below:
nix>     [  FAILED  ] ReplSessionTest.regression_9918
nix>     [  FAILED  ] ReplSessionTest.stack_vars
nix>      2 FAILED TESTS

Specific implementation might look like something like XORing a random value into symbols during comparisons if a specific diagnostic option is enabled. Or maybe https://github.com/NixOS/nix/pull/8711, but that implementation is somewhat of a hack.

We reproduced the below bug in building from a different nixpkgs, where the ordering of the attr sets from the debugger changed. Although the answer to *that* is "sort the debugger output", we should have a way to ferret out similar sorts of nondeterminism due to Symbol pointer comparisons in a way that will cause it to fail way earlier. ``` nix> With diff: nix> @@ -11,5 +11,5 @@ nix> Output "" nix> Output "Env level 2" nix> -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation" nix> +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation" nix> Command "meow" nix> Output "2" nix> @@ -27,5 +27,5 @@ nix> Output "" nix> Output "Env level 1" nix> -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation" nix> +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation" nix> Command ":c" nix> Output "trace: before inner break" nix> @@ -41,5 +41,5 @@ nix> Output "" nix> Output "Env level 2" nix> -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation" nix> +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation" nix> Command "meow'" nix> Output "3" nix> @@ -57,3 +57,3 @@ nix> Output "" nix> Output "Env level 1" nix> -Output "builtins true false null fromTOML fetchTree fetchTarball fetchGit fetchMercurial scopedImport import isNull b reak abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString derivation"\n nix> +Output "builtins true false null scopedImport import isNull break abort throw derivationStrict placeholder baseNameOf dirOf removeAttrs map toString fetchMercurial fetchTree fetchTarball fetchGit fromTOML derivation"\n nix> [ FAILED ] ReplSessionTest.stack_vars (32 ms) nix> [----------] 5 tests from ReplSessionTest (118 ms total) nix> [----------] Global test environment tear-down nix> [==========] 5 tests from 1 test suite ran. (118 ms total) nix> [ PASSED ] 3 tests. nix> [ FAILED ] 2 tests, listed below: nix> [ FAILED ] ReplSessionTest.regression_9918 nix> [ FAILED ] ReplSessionTest.stack_vars nix> 2 FAILED TESTS ``` Specific implementation might look like something like XORing a random value into symbols during comparisons if a specific diagnostic option is enabled. Or maybe https://github.com/NixOS/nix/pull/8711, but that implementation is somewhat of a hack.