Skip to content

Conversation

@bbenne10
Copy link
Contributor

@bbenne10 bbenne10 commented Aug 9, 2025

Depends on #593, but goes much farther.
Switches entirely to bats + bats-assert for testing.

This gets rid of pytest and allows us to be truly "bash-first".
Additionally, the logic of the gc tests is changed a bit to avoid gc'ing at all.
We know where the gc roots are created so we just check if everything has been wired up correctly. This makes test much less intrusive in non-ci scenarios.

Additionally, I believe it should run faster overall by way of doing much less work.

If you are not sure of the lifecycle:

  • setup in any test file is akin to a function-scoped pytest fixture pre-yield: it is run every test and acts as an initializer.
  • teardown in any test file is akin to function-scoped pytest fixute post-yield: runs post every test and acts as a cleaner.

This should be enough to understand everything here, but there's more. If you want to know, the docs are here, but feel free to ask me here; they're a bit all over the place and useful things are sometimes difficult to find.

Tagging @fidgetingbits for situational awareness and possible review.

@bbenne10 bbenne10 requested a review from Mic92 August 9, 2025 12:55
@bbenne10 bbenne10 self-assigned this Aug 9, 2025
@bbenne10 bbenne10 changed the title Switch to bats over pytest; Switch to bats over pytest Aug 9, 2025
@bbenne10
Copy link
Contributor Author

bbenne10 commented Aug 9, 2025

Note that these tests are something like 40% faster overall than the pytest suite. I'll take that :)

shellcheck
;
})
++ (builtins.attrValues (lib.attrsets.filterAttrs (name: _val: name != "direnv-stdlib") test_pkgs))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because direnv-stdlib is the result of fetchurl, it's a single-file in the store. This causes the symlinking to create the package environment to abort hard on it. We just filter it here so it is available when setting DIRENV_STDLIB but not in the package set.

Copy link
Contributor

@fidgetingbits fidgetingbits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments from me, with the caveat that I didn't really try to understand how all the tests actually work in detail, just took a quick look and commented on what failed locally. Overall changes look good to me though. Seems much more natural to read than the pytest code was.


function assert_gcroot {
profile_path=$(find "$TESTDIR/.direnv" -type l | head -n 1)
run bats_pipe find /nix/var/nix/gcroots/auto/ -type l -printf "%l\n" \| grep -q "$profile_path"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why this fails on my system and not on yours or CI, but I get a $status == 141 error for the 4 tests that call assert_gcroot. 141 corresponds to a pipefail. This is often apparently because there is more to be written to the pipe before that isn't fully read? But not exactly sure why it'd be happening here. The default behavior of bats_pipe is to return in $status the value of the first command with a non-zero status value, and I guess 141 is coming from find piping into grep(?) so even though grep succeeds we still see the error. If I use run bats_pipe -1 (which indicates taking the error code from the Nth (zero-counted afaict) argument, then it seems to ignore the pipefail in my case and base it off greps status code. I've tested that if I purposefully cause grep to fail with a bad value, then I do get the test failure.

So I think I would suggest maybe adding -1 to bats_pipe here, with the caveat being in the unlikely event this ever changed to have additional |'s the value would have to get bumped I guess, but also happy if someone knows a better solution to this problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the best of my knowledge, grep only exits 0, 1, or 2 (and only 0 or 1 if -q is provided). Do you have that folder? What's in it? Is your nix version recent? Where does the symlink inside $TESTDIR/.direnv point? (You'll need to modify the tests to comment or remove the rm -rf $TESTDIR from util.bash's _common_teardown and probably will want to replace it with an echo of the raw $TESTDIR value).

Is this a nixos machine you're on? If no - is it a mac?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My saying to use bats_pipe -1 was meant to explain in part it's not actually grep failing. By passing -1 you're telling bats to use grep's $status value instead of any earlier failure value in $status. The command does actually work with the expected result when using bats_pipe -1.

I'm using nixos, I do have the folder and it's a regular nix gcroots folder. Nix version is:

❯ nix --version
nix (Nix) 2.31pre20250712_b1245123

which is what lead to my fixing the version tests in my other PR.

❯ ls -l /tmp/nix-direnv.qQF2Rj/.direnv/
drwxr-xr-x   - aa users 11  8月 22:17 bin
drwxr-xr-x   - aa users 11  8月 22:17 flake-inputs
lrwxrwxrwx   - aa users 11  8月 22:17 flake-profile-a5d5b61aa8a61b7d9d765e1daf971a9a578f1cfa -> /nix/store/amwjf6d2kkmb6vxnl8ia87b8384acc8s-nix-shell-env
.rw-r--r-- 57k aa users 11  8月 22:17 flake-profile-a5d5b61aa8a61b7d9d765e1daf971a9a578f1cfa.rc

This is what the profile output path was in one run, and the contents.

❯ ls -l /tmp/nix-direnv.qQF2Rj/.direnv/flake-inputs/p58d2j0ac7zvja5jl14xzbc19fakjxh2-source
.r--r--r-- 1.2k root root  1  1月  1970 allSystems.nix
.r--r--r-- 1.9k root root  1  1月  1970 check-utils.nix
.r--r--r--   44 root root  1  1月  1970 default.nix
dr-xr-xr-x    - root root  1  1月  1970 examples
.r--r--r-- 1.0k root root  1  1月  1970 filterPackages.nix
.r--r--r--  546 root root  1  1月  1970 flake.lock
.r--r--r--  780 root root  1  1月  1970 flake.nix
.r--r--r--  982 root root  1  1月  1970 flattenTree.nix
.r--r--r-- 5.9k root root  1  1月  1970 lib.nix
.r--r--r-- 1.1k root root  1  1月  1970 LICENSE
.r--r--r-- 5.5k root root  1  1月  1970 README.md
.r--r--r-- 1.8k root root  1  1月  1970 simpleFlake.nix

Running the command manually works as expected:

❯ find /nix/var/nix/gcroots/auto -type l -printf "%l\n" | grep /tmp/nix-direnv.qQF2Rj/.direnv/flake-inputs/p58d2j0ac7zvja5jl14xzbc19fakjxh2-source
/tmp/nix-direnv.qQF2Rj/.direnv/flake-inputs/p58d2j0ac7zvja5jl14xzbc19fakjxh2-source

No errors or anything from just running find on that folder either, and $? is 0.

But if I run with bats I get the pipe error (includes the extra echo I put in for profile_path):

❯ bats -x --verbose-run ./tests/test_gc.bats
test_gc.bats
 ✗ use_nix_no_strict
   (from function `assert_success' in file /nix/store/pgbgss9b4qr206dk3v1zl370z5v8456j-bats-assert//bats-assert/src/assert_success.bash, line 45,
    from function `assert_gcroot' in file tests/test_gc.bats, line 26,
    in test file tests/test_gc.bats, line 52)
     `assert_gcroot' failed
   $ [test_gc.bats, line 5]
   $ load "util"
   $ _common_setup
   $$ [util.bash, line 2]
   $$ shopt -s globstar
   $$ bats_require_minimum_version 1.5.0
   $$ bats_load_library bats-support
   $$$$$$ [load.bash, line 9]
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/output.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/error.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/lang.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$ [util.bash, line 5]
   $$ bats_load_library bats-assert
   $$$$$$ [load.bash, line 22]
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ [load.bash, line 23]
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/refute.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/assert_equal.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/assert_not_equal.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/assert_success.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/assert_failure.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/assert_output.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/refute_output.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/assert_line.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/refute_line.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/assert_regex.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$$$$$ source "$(dirname "${BASH_SOURCE[0]}")/src/refute_regex.bash"
   $$$$$$ dirname "${BASH_SOURCE[0]}"
   $$ [util.bash, line 7]
   $$ TESTDIR=
   $$ TESTDIR=$(mktemp -d -t nix-direnv.XXXXXX)
   $$ mktemp -d -t nix-direnv.XXXXXX
   $$ export TESTDIR
   $$ export DIRENV_LOG_FORMAT="direnv: %s"
   $$ export NIX_USER_CONF_DIRS="$HOME/.config/nix/nix.conf:$NIX_USER_CONF_DIRS"
   $$ export HOME=$TESTDIR/home
   $$ unset XDG_DATA_HOME
   $$ unset XDG_CONFIG_HOME
   $$ cp "$BATS_TEST_DIRNAME"/testenv/* "$TESTDIR/"
   $ [test_gc.bats, line 49]
   $ "$@"
   $ write_envrc "use nix"
   $$ [util.bash, line 24]
   $$ echo "source $DIRENVRC" > "$TESTDIR/.envrc"
   $$ echo -e "\n$*" >> "$TESTDIR/.envrc"
   $$ direnv allow "$TESTDIR"
   $ [test_gc.bats, line 51]
   $ assert_run_output
   $$ [test_gc.bats, line 17]
   $$ run_in_direnv hello
   $$$ [util.bash, line 30]
   $$$ run --separate-stderr direnv exec "$TESTDIR" sh -c "$@"
   Hello, world!
   stderr:
   direnv: loading /tmp/nix-direnv.actqiO/.envrc
   direnv: using nix
   direnv: nix-direnv: Renewed cache
   Executing shellHook.
   $$$ assert_success
   $$$$ [assert_success.bash, line 33]
   $$$$ : "${output?}"
   $$$$ : "${status?}"
   $$$$ (( status != 0 ))
   $$$ [util.bash, line 32]
   $$$ run direnv exec "$TESTDIR" sh -c "$@"
   direnv: loading /tmp/nix-direnv.actqiO/.envrc
   direnv: using nix
   direnv: nix-direnv: Using cached dev shell
   Executing shellHook.
   Hello, world!
   $$$ assert_success
   $$$$ [assert_success.bash, line 33]
   $$$$ : "${output?}"
   $$$$ : "${status?}"
   $$$$ (( status != 0 ))
   $$$ [util.bash, line 34]
   $$$ assert_stderr -p "Renewed cache"
   $$$$ [assert_output.bash, line 157]
   $$$$ __assert_stream "$@"
   $$$$$ [assert_output.bash, line 161]
   $$$$$ local -r caller=${FUNCNAME[1]}
   $$$$$ local -r stream_type=${caller/assert_/}
   $$$$$ local -i is_mode_partial=0
   $$$$$ local -i is_mode_regexp=0
   $$$$$ local -i is_mode_nonempty=0
   $$$$$ local -i use_stdin=0
   $$$$$ [[ ${stream_type} == "output" ]]
   $$$$$ [[ ${stream_type} == "stderr" ]]
   $$$$$ : "${stderr?}"
   $$$$$ local -r stream="${!stream_type}"
   $$$$$ (( $# == 0 ))
   $$$$$ (( $# > 0 ))
   $$$$$ case "$1" in
   $$$$$ is_mode_partial=1
   $$$$$ shift
   $$$$$ (( $# > 0 ))
   $$$$$ case "$1" in
   $$$$$ break
   $$$$$ (( is_mode_partial ))
   $$$$$ (( is_mode_regexp ))
   $$$$$ local expected
   $$$$$ (( use_stdin ))
   $$$$$ expected="${1-}"
   $$$$$ (( is_mode_nonempty ))
   $$$$$ (( is_mode_regexp ))
   $$$$$ (( is_mode_partial ))
   $$$$$ [[ $stream != *"$expected"* ]]
   $$ [test_gc.bats, line 18]
   $$ assert_output -p "Hello, world"
   $$$ [assert_output.bash, line 125]
   $$$ __assert_stream "$@"
   $$$$ [assert_output.bash, line 161]
   $$$$ local -r caller=${FUNCNAME[1]}
   $$$$ local -r stream_type=${caller/assert_/}
   $$$$ local -i is_mode_partial=0
   $$$$ local -i is_mode_regexp=0
   $$$$ local -i is_mode_nonempty=0
   $$$$ local -i use_stdin=0
   $$$$ [[ ${stream_type} == "output" ]]
   $$$$ : "${output?}"
   $$$$ local -r stream="${!stream_type}"
   $$$$ (( $# == 0 ))
   $$$$ (( $# > 0 ))
   $$$$ case "$1" in
   $$$$ is_mode_partial=1
   $$$$ shift
   $$$$ (( $# > 0 ))
   $$$$ case "$1" in
   $$$$ break
   $$$$ (( is_mode_partial ))
   $$$$ (( is_mode_regexp ))
   $$$$ local expected
   $$$$ (( use_stdin ))
   $$$$ expected="${1-}"
   $$$$ (( is_mode_nonempty ))
   $$$$ (( is_mode_regexp ))
   $$$$ (( is_mode_partial ))
   $$$$ [[ $stream != *"$expected"* ]]
   $$ [test_gc.bats, line 19]
   $$ assert_stderr -p "Executing shellHook"
   $$$ [assert_output.bash, line 157]
   $$$ __assert_stream "$@"
   $$$$ [assert_output.bash, line 161]
   $$$$ local -r caller=${FUNCNAME[1]}
   $$$$ local -r stream_type=${caller/assert_/}
   $$$$ local -i is_mode_partial=0
   $$$$ local -i is_mode_regexp=0
   $$$$ local -i is_mode_nonempty=0
   $$$$ local -i use_stdin=0
   $$$$ [[ ${stream_type} == "output" ]]
   $$$$ [[ ${stream_type} == "stderr" ]]
   $$$$ : "${stderr?}"
   $$$$ local -r stream="${!stream_type}"
   $$$$ (( $# == 0 ))
   $$$$ (( $# > 0 ))
   $$$$ case "$1" in
   $$$$ is_mode_partial=1
   $$$$ shift
   $$$$ (( $# > 0 ))
   $$$$ case "$1" in
   $$$$ break
   $$$$ (( is_mode_partial ))
   $$$$ (( is_mode_regexp ))
   $$$$ local expected
   $$$$ (( use_stdin ))
   $$$$ expected="${1-}"
   $$$$ (( is_mode_nonempty ))
   $$$$ (( is_mode_regexp ))
   $$$$ (( is_mode_partial ))
   $$$$ [[ $stream != *"$expected"* ]]
   $ [test_gc.bats, line 52]
   $ assert_gcroot
   $$ [test_gc.bats, line 23]
   $$ profile_path=$(find "$TESTDIR/.direnv" -type l | head -n 1)
   $$ find "$TESTDIR/.direnv" -type l
   $$ head -n 1
   $$ echo "profile_path: $profile_path"
   profile_path: /tmp/nix-direnv.actqiO/.direnv/nix-profile-25.05-086lky2c6izqghqc
   $$ run bats_pipe find /nix/var/nix/gcroots/auto/ -type l -printf "%l\n" \| grep -q "$profile_path"
   $$ assert_success
   $$$ [assert_success.bash, line 33]
   $$$ : "${output?}"
   $$$ : "${status?}"
   $$$ (( status != 0 ))
   $$$ batslib_decorate 'command failed'
   $$$ local -ir width=6
   $$$ batslib_print_kv_single "$width" 'status' "$status"
   $$$ fail
   $$$$ [output.bash, line 130]
   $$$$ local -ir col_width="$1"
   $$$$ [output.bash, line 274]
   $$$$ echo
   $$$$ shift
   $$$$ echo "-- $1 --"
   $$$$ (( $# != 0 ))
   $$$$ PATH="$BATS_SAVED_PATH" command cat -
   $$$$ [error.bash, line 39]
   $$$$ printf '%-*s : %s\n' "$col_width" "$1" "$2"
   $$$$ (( $# == 0 ))
   $$$$ shift 2
   $$$$ batslib_err
   $$$$ (( $# != 0 ))
   $$$ [assert_success.bash, line 39]
   $$$ batslib_print_kv_single_or_multi "$width" 'output' "$output"
   $$$$$ [output.bash, line 38]
   $$$$$ (( $# > 0 ))
   $$$$$ PATH="$BATS_SAVED_PATH" command cat -
   $$$$ [output.bash, line 183]
   $$$$ local -ir width="$1"
   $$$$ shift
   $$$$ local -a pairs=("$@")
   $$$$ local -a values=()

   -- command failed --
   status : 141
   $$$$ local -i i
   $$$$ ((i=1))
   $$$$ ((i < 2))
   $$$$ values+=("${pairs[$i]}")
   $$$$ ((i+=2 ))
   $$$$ ((i < 2))
   $$$$ batslib_is_single_line "${values[@]}"
   $$$$$ [output.bash, line 81]
   $$$$$ for string in "$@"
   $$$$$ (( $(batslib_count_lines "$string") > 1 ))
   $$$$$ batslib_count_lines "$string"
   $$$$$$ [output.bash, line 63]
   $$$$$$ local -i n_lines=0
   $$$$$$ local line
   $$$$$$ IFS='' read -r line
   $$$$$$ printf '%s' "$1"
   $$$$$$ [[ -n $line ]]
   $$$$$$ echo "$n_lines"
   $$$$$ return 0
   $$$$ [output.bash, line 193]
   $$$$ batslib_print_kv_single "$width" "${pairs[@]}"
   $$$$$ [output.bash, line 130]
   $$$$$ local -ir col_width="$1"
   $$$$$ shift
   $$$$$ (( $# != 0 ))
   $$$$$ printf '%-*s : %s\n' "$col_width" "$1" "$2"
   output :
   $$$$$ shift 2
   $$$$$ (( $# != 0 ))
   $$$ [assert_success.bash, line 40]
   $$$ [[ -n "${stderr-}" ]]
   $$$ batslib_print_kv_single_or_multi "$width" 'stderr' "$stderr"
   $$$$ [output.bash, line 183]
   $$$$ local -ir width="$1"
   $$$$ shift
   $$$$ local -a pairs=("$@")
   $$$$ local -a values=()
   $$$$ local -i i
   $$$$ ((i=1))
   $$$$ ((i < 2))
   $$$$ values+=("${pairs[$i]}")
   $$$$ ((i+=2 ))
   $$$$ ((i < 2))
   $$$$ batslib_is_single_line "${values[@]}"
   $$$$$ [output.bash, line 81]
   $$$$$ for string in "$@"
   $$$$$ (( $(batslib_count_lines "$string") > 1 ))
   $$$$$ batslib_count_lines "$string"
   $$$$$$ [output.bash, line 63]
   $$$$$$ local -i n_lines=0
   $$$$$$ local line
   $$$$$$ IFS='' read -r line
   $$$$$$ printf '%s' "$1"
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ [[ -n $line ]]
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ [[ -n $line ]]
   $$$$$$ echo "$n_lines"
   $$$$$ return 1
   $$$$ [output.bash, line 195]
   $$$$ local -i i
   $$$$ ((i=1))
   $$$$ ((i < 2))
   $$$$ pairs[$i]="$(batslib_prefix < <(printf '%s' "${pairs[$i]}"))"
   $$$$ batslib_prefix < <(printf '%s' "${pairs[$i]}")
   $$$$ printf '%s' "${pairs[$i]}"
   $$$$$ [output.bash, line 216]
   $$$$$ local -r prefix="${1:-  }"
   $$$$$ local line
   $$$$$ IFS='' read -r line
   $$$$$ printf '%s%s\n' "$prefix" "$line"
   $$$$$ IFS='' read -r line
   $$$$$ printf '%s%s\n' "$prefix" "$line"
   $$$$$ IFS='' read -r line
   $$$$$ printf '%s%s\n' "$prefix" "$line"
   $$$$$ IFS='' read -r line
   $$$$$ [[ -n $line ]]
   $$$$$ printf '%s%s\n' "$prefix" "$line"
   $$$$$ IFS='' read -r line
   $$$$$ [[ -n $line ]]
   $$$$ ((i+=2 ))
   $$$$ ((i < 2))
   $$$$ batslib_print_kv_multi "${pairs[@]}"
   $$$$$ [output.bash, line 154]
   $$$$$ (( $# != 0 ))
   $$$$$ printf '%s (%d lines):\n' "$1" "$(batslib_count_lines "$2")"
   $$$$$ batslib_count_lines "$2"
   $$$$$$ [output.bash, line 63]
   $$$$$$ local -i n_lines=0
   $$$$$$ local line
   $$$$$$ IFS='' read -r line
   $$$$$$ printf '%s' "$1"
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ [[ -n $line ]]
   $$$$$$ (( ++n_lines ))
   $$$$$$ IFS='' read -r line
   $$$$$$ [[ -n $line ]]
   $$$$$$ echo "$n_lines"
   stderr (4 lines):
   $$$$$ printf '%s\n' "$2"
     direnv: loading /tmp/nix-direnv.actqiO/.envrc
     direnv: using nix
     direnv: nix-direnv: Renewed cache
     Executing shellHook.
   $$$$$ shift 2
   $$$$$ (( $# != 0 ))
   $$$$ echo '--'
   --
   $$$$ echo

   $$$$ [error.bash, line 40]
   $$$$ return 1

If I add bats_pipe -1:

❯ bats ./tests/test_gc.bats
test_gc.bats
 ✓ use_nix_no_strict
 ✓ use_nix_strict
 ✓ use_flake_no_strict
 ✓ use_flake_strict

4 tests, 0 failures

If I use run bash -c 'find /nix/var/nix/gcroots/auto/ -type l -printf "%l\n" | grep -q "$profile_path"' instead of bats_pipe it also passes, so seems specific to using bats_pipe.

Just to confirm that it's not grep failing (as I mentioned above), if I use run bats_pipe find /nix/var/nix/gcroots/auto/ -type l -printf "%l\n" \| echo "foo" I still get the 141 error. It's not the folder itself as I tested with just a regular empty folder and find on it and it also failed. I then tested with ls instead of find and it also fails. Oddly run bats_pipe echo foo \| echo foo is okay.

These tests do fail when run via nix run too.

I don't have time to try to debug bats_pipe atm, but maybe you see something in the above that gives a hint what's going on. I should have time tomorrow to try it on a different nixos box to see if I run into the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should've been clearer. I am resistant to using bats_pipe -1 here because I don't want to write data to a closed pipe at all. That seems a poor design that we should avoid rather than sidestep with tooling, especially if neither CI nor myself can replicate. I'm not entirely sure where the problem lies at the moment, but I will make some time to figure it out soon.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. fwiw I've tested on three other systems and can't replicate the 141 status failures on any of them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw I just took a quick through an strace log to confirm the EPIPE happens the first time find tries to write to stdout after grep process has existed. grep -q will prematurely exit on the first match, which could happen while find is still running. Why this only triggers on the one box, and not the others, I'm not exactly sure yet. But I confirm if I run without grep -q it doesn't EPIPE.

If I look at the strace on the system it works on I see find exits before grep, so never tries to write the pipe after grep has already exited.

I suppose it could be how long the first command takes on my failing system vs the other (maybe slow disks, more files, etc). My gcroots/auto folder on the failing system has ~1600 entries. The working systems all have ~200-300. Seems weird with so few files for that to be the cause though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ THIS is the investigation that makes me comfortable implementing this with bats_pipe -1. This is way beyond the depth I expected from you! I hadn't found time to do the investigation myself and I truly appreciate the dedication!

I am comfortable saying "The pipe fills and grep -q may exit early as soon as it finds a match without emptying the pipe, even before the associated find completes". Thanks. I will add the -1 and a comment explaining everything and I think we can move on.

@bbenne10 bbenne10 force-pushed the bats_testing branch 2 times, most recently from 2bb4e74 to d83dc7f Compare August 10, 2025 17:45
@bbenne10
Copy link
Contributor Author

I think this is ready for another round of review, if anyone finds some time.

@Mic92 Mic92 requested a review from Copilot August 18, 2025 08:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR migrates the test suite from pytest (Python) to bats (Bash Automated Testing System) with bats-assert library. This change aligns the testing framework with the project's "bash-first" philosophy and removes Python dependencies from the test infrastructure.

Key changes:

  • Complete replacement of Python-based pytest tests with Bash-based bats tests
  • Updated test logic for garbage collection tests to check GC root creation instead of performing actual garbage collection
  • New test infrastructure using bats libraries and shell utilities

Reviewed Changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
treefmt.nix Removed Python linting tools (mypy, ruff) and added bats test files to shell formatting
tests/util.bash New utility functions for bats test setup, teardown, and common test operations
tests/test_versions.bats New bats tests for version checking functionality
tests/test_use_nix.py → tests/test_use_nix.bats Converted Python tests to bats format for nix usage testing
tests/test_gc.py → tests/test_gc.bats Converted Python GC tests to bats with modified logic to check GC roots instead of running collection
tests/nix/bats-*.nix New Nix derivations for bats testing libraries
tests/default.nix New test runner configuration using bats instead of pytest
Multiple Python files Removed Python test infrastructure files
shell.nix, flake.nix Updated to support bats testing environment
direnvrc Fixed version parsing to handle pre-release versions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

This removes python entirely from the stack.
Yet to run the tests on macOS, but they're buildable by way of nix
wrapping. Worst case scenario, you cannot run them from the devshell
but `nix run .#test-runner-<latest||stable>` should work okay.
@bbenne10
Copy link
Contributor Author

I'm going to merge this tomorrow afternoon EDT if no one has more thoughts. Thanks!

@bbenne10 bbenne10 added this pull request to the merge queue Aug 28, 2025
Merged via the queue into master with commit c0270d9 Aug 28, 2025
5 checks passed
@bbenne10 bbenne10 deleted the bats_testing branch August 28, 2025 18:55
@bbenne10 bbenne10 mentioned this pull request Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants