Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with using load in teardown #609

Closed
colleeneb opened this issue Jun 8, 2022 · 5 comments · Fixed by #612
Closed

Issue with using load in teardown #609

colleeneb opened this issue Jun 8, 2022 · 5 comments · Fixed by #612
Labels
Component: Bash Code Everything regarding the bash code Priority: Critical Broken behavior in nearly all environments, e.g. wrong test results, internal bats error Size: Medium Changes in the same file Status: Confirmed The reproducer worked as described Type: Bug

Comments

@colleeneb
Copy link

Describe the bug
If we use load in the teardown, bats dies in the run and does not complete.

To Reproduce
Steps to reproduce the behavior:

Given this .bats file:

> cat t.bats 
teardown() {
      load ./common_functions.bash
 }

@test test2 {
     g++ test.cpp
    ./a.out
}

and this test.cpp:

> cat test.cpp 
int main() {
      return 1;
}

and this common_functions.bash:

> cat common_functions.bash 
function test_exit
{
        echo "Hi"
}

And then run bats. What I'm seeing is that it dies before printing what passed or failed:

> ./bin/bats -t t.bats 
1..1
# bats warning: Executed 0 instead of expected 1 tests

Expected behavior
Since test.cpp just returns 1, I expect bats to say that the test failed and exit cleanly. If we remove the teardown() function from t.bats it does work as expected:

> ./bin/bats -t t.bats 
1..1
not ok 1 test2
# (in test file t.bats, line 5)
#   `./a.out' failed

Environment (please complete the following information):

  • Bats Version: 61abd09
  • OS: Linux
  • Bash version: GNU bash, version 4.4.23(1)-release (x86_64-suse-linux-gnu)

Additional context
If I'm doing something wrong in the code, let me know! But I thought it should be ok.

@colleeneb colleeneb added Priority: NeedsTriage Issue has not been vetted yet Type: Bug labels Jun 8, 2022
@martin-schulze-vireso
Copy link
Member

Sounds similar to bats-core/bats-file#40

@martin-schulze-vireso martin-schulze-vireso added Component: Bash Code Everything regarding the bash code Status: Unconfirmend No reproducer was provided or the reproducer did not work for maintainers. labels Jun 8, 2022
@martin-schulze-vireso
Copy link
Member

martin-schulze-vireso commented Jun 8, 2022

Does this only happen with -t? Did it work with a previous Bats version?

@colleeneb
Copy link
Author

Thanks a lot for responding!

It happens without -t as well:

> ./bin/bats t.bats 
t.bats
   test2                                                                                                                                                                                                      1/1
   bats warning: Executed 0 instead of expected 1 tests

1 test, 0 failures, 1 not run

It did work for a previous version, and I just ran git bisect to find the commit. The result was:

2469ef2cd5b957eb57d1891952352182db2b2277 is the first bad commit
commit 2469ef2cd5b957eb57d1891952352182db2b2277
Author: Nelo-T. Wallus <nelo@wallus.de>
Date:   Mon Jan 10 15:48:57 2022 +0100

    Rewrite BATS_LIB_PATH approach with clearer helper functions

 lib/bats-core/test_functions.bash               | 216 +++++++++++++++++-------
 test/bats.bats                                  | 108 +++++++++++-
 test/fixtures/bats/find_library_helper.bats     |   5 +
 test/fixtures/bats/find_library_helper_err.bats |   4 +
 4 files changed, 269 insertions(+), 64 deletions(-)
 create mode 100644 test/fixtures/bats/find_library_helper.bats
 create mode 100644 test/fixtures/bats/find_library_helper_err.bats

I'll try to look through to see if anything about that commit pops out, but I'm not a bash expert so it will take time. :)

@martin-schulze-vireso martin-schulze-vireso added Priority: Critical Broken behavior in nearly all environments, e.g. wrong test results, internal bats error Status: Confirmed The reproducer worked as described Size: Medium Changes in the same file and removed Status: Unconfirmend No reproducer was provided or the reproducer did not work for maintainers. Priority: NeedsTriage Issue has not been vetted yet labels Jun 15, 2022
@martin-schulze-vireso
Copy link
Member

Okay, I think this reproducer neatly sums up the problem:

#!/usr/bin/env bash

fun() {
    true
    return
}

print() {
    fun
    echo "$1: $?"
}

print FREE
trap 'print EXIT' EXIT

false # uncomment to get exit without error

executing this prints:

FREE: 0
EXIT: 1

Swapping the two occurrences of false and true will flip around the result too:

FREE: 1
EXIT: 0

This shows that return without parameter in an exit trap will always return the value the trap was entered with. The commit you bisected introduced the first return without parameter which leads to an explicit exit in load because bats_load_safe's return code got overwritten.

martin-schulze-vireso added a commit to martin-schulze-vireso/bats-core that referenced this issue Jun 15, 2022
martin-schulze-vireso added a commit to martin-schulze-vireso/bats-core that referenced this issue Jun 15, 2022
When run from the exit trap, `return` without parameter would
return the exit code the trap was called with instead of the last
command's before `return`.
@colleeneb
Copy link
Author

Thank you so much! For this fix and also generally thanks for bats, we use it in our test set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Bash Code Everything regarding the bash code Priority: Critical Broken behavior in nearly all environments, e.g. wrong test results, internal bats error Size: Medium Changes in the same file Status: Confirmed The reproducer worked as described Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants