From b63136687006c38e1cc5ea9c57a9466379eea9f1 Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Thu, 18 Jul 2024 16:28:33 +0300 Subject: [PATCH 1/3] test.testers: add failing tests for testers.testEqualContents This change adds tests for tests.testEqualContents to demonstrate that the current implementation is failing in some cases. --- pkgs/build-support/testers/test/default.nix | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/pkgs/build-support/testers/test/default.nix b/pkgs/build-support/testers/test/default.nix index a815fe63e416..ae7bbe810fa9 100644 --- a/pkgs/build-support/testers/test/default.nix +++ b/pkgs/build-support/testers/test/default.nix @@ -99,19 +99,35 @@ lib.recurseIntoAttrs { }; testEqualContents = lib.recurseIntoAttrs { - happy = testers.testEqualContents { + equalDir = testers.testEqualContents { assertion = "The same directory contents at different paths are recognized as equal"; expected = runCommand "expected" {} '' mkdir -p $out/c echo a >$out/a echo b >$out/b echo d >$out/c/d + echo e >$out/e + chmod a+x $out/e ''; actual = runCommand "actual" {} '' mkdir -p $out/c echo a >$out/a echo b >$out/b echo d >$out/c/d + echo e >$out/e + chmod a+x $out/e + ''; + }; + + equalExe = testers.testEqualContents { + assertion = "The same executable file contents at different paths are recognized as equal"; + expected = runCommand "expected" { } '' + echo test >$out + chmod a+x $out + ''; + actual = runCommand "actual" { } '' + echo test >$out + chmod a+x $out ''; }; From fc5c829532a9a275fe51959b1d0158622ee69ae6 Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Thu, 18 Jul 2024 17:01:35 +0300 Subject: [PATCH 2/3] testers.testEqualContents: use diffoscope instead of diffing find output Before this change, testers.testEqualContents implementation had several bugs (e.g. executables at different paths were not considered equal). So we switch to diffoscope that that is designed to handle exactly these kinds of comparisons and gives more insights into the differences in the output. --- pkgs/build-support/testers/default.nix | 27 ++++++++------------------ 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/pkgs/build-support/testers/default.nix b/pkgs/build-support/testers/default.nix index 516ab90db503..20d2957134c3 100644 --- a/pkgs/build-support/testers/default.nix +++ b/pkgs/build-support/testers/default.nix @@ -1,4 +1,4 @@ -{ pkgs, pkgsLinux, buildPackages, lib, callPackage, runCommand, stdenv, substituteAll, testers }: +{ pkgs, pkgsLinux, buildPackages, diffoscopeMinimal, lib, callPackage, runCommand, stdenv, substituteAll, testers }: # Documentation is in doc/build-helpers/testers.chapter.md { # See https://nixos.org/manual/nixpkgs/unstable/#tester-lycheeLinkCheck @@ -27,33 +27,22 @@ expected, }: runCommand "equal-contents-${lib.strings.toLower assertion}" { inherit assertion actual expected; + nativeBuildInputs = [ diffoscopeMinimal ]; } '' echo "Checking:" - echo "$assertion" - if ! diff -U5 -r "$actual" "$expected" --color=always + printf '%s\n' "$assertion" + if ! diffoscope --no-progress --text-color=always --exclude-directory-metadata=no -- "$actual" "$expected" then echo echo 'Contents must be equal, but were not!' echo echo "+: expected, at $expected" echo "-: unexpected, at $actual" - exit 1 + false else - find "$expected" -type f -executable > expected-executables | sort - find "$actual" -type f -executable > actual-executables | sort - if ! diff -U0 actual-executables expected-executables --color=always - then - echo - echo "Contents must be equal, but some files' executable bits don't match" - echo - echo "+: make this file executable in the actual contents" - echo "-: make this file non-executable in the actual contents" - exit 1 - else - echo "expected $expected and actual $actual match." - echo 'OK' - touch $out - fi + echo "expected $expected and actual $actual match." + echo OK + touch -- "$out" fi ''; From b40d043d5a2a1fb473f1c6c853dac444ceadab62 Mon Sep 17 00:00:00 2001 From: Ivan Trubach Date: Thu, 18 Jul 2024 18:34:26 +0300 Subject: [PATCH 3/3] test.testers: update tests for testers.testEqualContents Updates tests for testers.testEqualContents with diffoscope and fixes some bugs in tests (e.g. fileDiff always succeed because subshell does not inherit errexit option). --- pkgs/build-support/testers/test/default.nix | 214 ++++++++++---------- 1 file changed, 108 insertions(+), 106 deletions(-) diff --git a/pkgs/build-support/testers/test/default.nix b/pkgs/build-support/testers/test/default.nix index ae7bbe810fa9..8e8886342f1e 100644 --- a/pkgs/build-support/testers/test/default.nix +++ b/pkgs/build-support/testers/test/default.nix @@ -1,4 +1,4 @@ -{ testers, lib, pkgs, hello, runCommand, ... }: +{ testers, lib, pkgs, hello, runCommand, emptyFile, emptyDirectory, ... }: let pkgs-with-overlay = pkgs.extend(final: prev: { proof-of-overlay-hello = prev.hello; @@ -101,131 +101,133 @@ lib.recurseIntoAttrs { testEqualContents = lib.recurseIntoAttrs { equalDir = testers.testEqualContents { assertion = "The same directory contents at different paths are recognized as equal"; - expected = runCommand "expected" {} '' - mkdir -p $out/c - echo a >$out/a - echo b >$out/b - echo d >$out/c/d - echo e >$out/e - chmod a+x $out/e + expected = runCommand "expected" { } '' + mkdir -p -- "$out/c" + echo a >"$out/a" + echo b >"$out/b" + echo d >"$out/c/d" + echo e >"$out/e" + chmod a+x -- "$out/e" ''; - actual = runCommand "actual" {} '' - mkdir -p $out/c - echo a >$out/a - echo b >$out/b - echo d >$out/c/d - echo e >$out/e - chmod a+x $out/e + actual = runCommand "actual" { } '' + mkdir -p -- "$out/c" + echo a >"$out/a" + echo b >"$out/b" + echo d >"$out/c/d" + echo e >"$out/e" + chmod a+x -- "$out/e" ''; }; + fileMissing = testers.testBuildFailure ( + testers.testEqualContents { + assertion = "Directories with different file list are not recognized as equal"; + expected = runCommand "expected" { } '' + mkdir -p -- "$out/c" + echo a >"$out/a" + echo b >"$out/b" + echo d >"$out/c/d" + ''; + actual = runCommand "actual" { } '' + mkdir -p -- "$out/c" + echo a >"$out/a" + echo d >"$out/c/d" + ''; + } + ); + equalExe = testers.testEqualContents { assertion = "The same executable file contents at different paths are recognized as equal"; expected = runCommand "expected" { } '' - echo test >$out - chmod a+x $out + echo test >"$out" + chmod a+x -- "$out" ''; actual = runCommand "actual" { } '' - echo test >$out - chmod a+x $out + echo test >"$out" + chmod a+x -- "$out" ''; }; - unequalExe = - runCommand "testEqualContents-unequalExe" { - log = testers.testBuildFailure (testers.testEqualContents { - assertion = "The same directory contents at different paths are recognized as equal"; - expected = runCommand "expected" {} '' - mkdir -p $out/c - echo a >$out/a - chmod a+x $out/a - echo b >$out/b - echo d >$out/c/d - ''; - actual = runCommand "actual" {} '' - mkdir -p $out/c - echo a >$out/a - echo b >$out/b - chmod a+x $out/b - echo d >$out/c/d - ''; - }); - } '' - ( - set -x - grep -F -- "executable bits don't match" $log/testBuildFailure.log - grep -E -- '+.*-actual/a' $log/testBuildFailure.log - grep -E -- '-.*-actual/b' $log/testBuildFailure.log - grep -F -- "--- actual-executables" $log/testBuildFailure.log - grep -F -- "+++ expected-executables" $log/testBuildFailure.log - ) || { - echo "Test failed: could not find pattern in build log $log" - exit 1 - } - echo 'All good.' - touch $out - ''; + unequalExe = testers.testBuildFailure ( + testers.testEqualContents { + assertion = "Different file mode bits are not recognized as equal"; + expected = runCommand "expected" { } '' + touch -- "$out" + chmod a+x -- "$out" + ''; + actual = runCommand "actual" { } '' + touch -- "$out" + ''; + } + ); + + unequalExeInDir = testers.testBuildFailure ( + testers.testEqualContents { + assertion = "Different file mode bits are not recognized as equal in directory"; + expected = runCommand "expected" { } '' + mkdir -p -- "$out/a" + echo b >"$out/b" + chmod a+x -- "$out/b" + ''; + actual = runCommand "actual" { } '' + mkdir -p -- "$out/a" + echo b >"$out/b" + ''; + } + ); + + nonExistentPath = testers.testBuildFailure ( + testers.testEqualContents { + assertion = "Non existent paths are not recognized as equal"; + expected = "${emptyDirectory}/foo"; + actual = "${emptyDirectory}/bar"; + } + ); + + emptyFileAndDir = testers.testBuildFailure ( + testers.testEqualContents { + assertion = "Empty file and directory are not recognized as equal"; + expected = emptyFile; + actual = emptyDirectory; + } + ); fileDiff = - runCommand "testEqualContents-fileDiff" { - log = testers.testBuildFailure (testers.testEqualContents { - assertion = "The same directory contents at different paths are recognized as equal"; - expected = runCommand "expected" {} '' - mkdir -p $out/c - echo a >$out/a - echo b >$out/b - echo d >$out/c/d - ''; - actual = runCommand "actual" {} '' - mkdir -p $out/c - echo a >$out/a - echo B >$out/b - echo d >$out/c/d - ''; - }); - } '' + let + log = testers.testBuildFailure ( + testers.testEqualContents { + assertion = "Different files are not recognized as equal in subdirectories"; + expected = runCommand "expected" { } '' + mkdir -p -- "$out/b" + echo a >"$out/a" + echo EXPECTED >"$out/b/c" + ''; + actual = runCommand "actual" { } '' + mkdir -p "$out/b" + echo a >"$out/a" + echo ACTUAL >"$out/b/c" + ''; + } + ); + in + runCommand "testEqualContents-fileDiff" { inherit log; } '' ( set -x - grep -F -- "Contents must be equal but were not" $log/testBuildFailure.log - grep -E -- '+++ .*-actual/b' $log/testBuildFailure.log - grep -E -- '--- .*-actual/b' $log/testBuildFailure.log - grep -F -- "-B" $log/testBuildFailure.log - grep -F -- "+b" $log/testBuildFailure.log + # Note: use `&&` operator to chain commands because errexit (set -e) + # does not work in this context (even when set explicitly and with + # inherit_errexit), otherwise the subshell exits with the status of + # the last run command and ignores preceding failures. + grep -F -- 'Contents must be equal, but were not!' "$log/testBuildFailure.log" && + grep -E -- '\+\+\+ .*-expected/b/c' "$log/testBuildFailure.log" && + grep -E -- '--- .*-actual/b/c' "$log/testBuildFailure.log" && + grep -F -- -ACTUAL "$log/testBuildFailure.log" && + grep -F -- +EXPECTED "$log/testBuildFailure.log" ) || { echo "Test failed: could not find pattern in build log $log" - exit 1 + false } echo 'All good.' - touch $out - ''; - - fileMissing = - runCommand "testEqualContents-fileMissing" { - log = testers.testBuildFailure (testers.testEqualContents { - assertion = "The same directory contents at different paths are recognized as equal"; - expected = runCommand "expected" {} '' - mkdir -p $out/c - echo a >$out/a - echo b >$out/b - echo d >$out/c/d - ''; - actual = runCommand "actual" {} '' - mkdir -p $out/c - echo a >$out/a - echo d >$out/c/d - ''; - }); - } '' - ( - set -x - grep -F -- "Contents must be equal but were not" $log/testBuildFailure.log - grep -E -- 'Only in .*-expected: b' $log/testBuildFailure.log - ) || { - echo "Test failed: could not find pattern in build log $log" - exit 1 - } - echo 'All good.' - touch $out + touch -- "$out" ''; }; }