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

oneTimeTearDown()/tearDown() is being called an additional time at the end of execution #112

Open
vtereso opened this issue Feb 13, 2019 · 13 comments

Comments

@vtereso
Copy link

vtereso commented Feb 13, 2019

I have been pulling master and had functions that were executing within these hooks and saw what seems to be some unintended behavior. Example file as show below:

testSomething() {
    assertTrue "true" "true"
}

setUp() {
  echo "setup"
}
tearDown() {
  echo "Tear down"
}
oneTimeTearDown() {
  echo "One time tear down"
}
. shunit2

Output:

setup
testSomething
Tear down
One time tear down

Ran 1 test.

OK
Tear down
One time tear down
@vtereso
Copy link
Author

vtereso commented Feb 13, 2019

Looking into it, I only see warnings being raised if these functions do not exit successfully, but do not see why they would be called at the end of execution. PR for consideration: #113

@cbandy
Copy link

cbandy commented Feb 18, 2019

These are being called as part of the EXIT trap. Related to #108.

@vtereso
Copy link
Author

vtereso commented Feb 18, 2019

Right, I wasn't familiarized with the entire topology. Looked things over and there is still a fundamental flaw since the exit code at the end of the script gets caught by the trap on successful runs so the 0 signal triggers _shunit_cleanup and the following block

if command [ ${__shunit_clean} -eq ${SHUNIT_FALSE} ]; then
    # Ensure tear downs are only called once.
    __shunit_clean=${SHUNIT_TRUE}

    tearDown
    command [ $? -eq ${SHUNIT_TRUE} ] \
        || _shunit_warn "tearDown() returned non-zero return code."
    oneTimeTearDown
    command [ $? -eq ${SHUNIT_TRUE} ] \
        || _shunit_warn "oneTimeTearDown() returned non-zero return code."

    command rm -fr "${__shunit_tmpDir}"
  fi

will trigger the tear downs again. I don't know what @kward's perspective is and whether the idea was to have termination always happen from within _shunit_cleanup (migrate in _shunit_generateReport and relative code here for 0 signal) or to remove the trap '_shunit_cleanup EXIT' 0

@jaspalch
Copy link

jaspalch commented Apr 2, 2019

This can be used as sort of a hacky workaround:

tearDown() {
    #need this to suppress tearDown on script EXIT
    [[ "${_shunit_name_}" = 'EXIT' ]] && return 0

@swcline
Copy link

swcline commented Feb 13, 2020

I tried adding the above code inside tearDown() but get the following error - is there something else that needs to be enabled or set?

in oneTimeSetUp
in setUp
testOne
validateTestOrdinals_test.sh[34]: shunit_name: parameter not set

the test script:

#! /bin/sh
#assume this test script is in a subdirectory under the shunit directory; a sibling of examples

#Load test helpers.
. ../shunit2_test_helpers

testOne() {
assertTrue 0
}

testTwo() {
assertTrue 0
}

setUp() {
echo "in setUp"
}

tearDown() {
#need this to suppress tearDown on script EXIT
[[ "${shunit_name}" = 'EXIT' ]] && return 0
echo "in tearDown"
}

oneTimeSetUp() {
echo "in oneTimeSetUp"
}

oneTimeTearDown() {
echo "in oneTimeTearDown"
}

#Load and run shUnit2.
. ../shunit2

@swcline
Copy link

swcline commented Feb 13, 2020

My workaround is to create a directory in oneTimeSetUp and wrap the tear down code inside a check for that directory inside oneTimeTearDown - second time around, directory is not there, so code is skipped

torgnylyon added a commit to torgnylyon/shunit2 that referenced this issue May 2, 2020
postmodern added a commit to postmodern/ruby-install that referenced this issue Jun 5, 2021
* tearDown actually runs an additional time after all test_* functions
  have been called.
  kward/shunit2#112
postmodern added a commit to postmodern/chruby that referenced this issue Sep 12, 2021
* GitHub Action's macOS image provides a different version of shunit2.
* shnit2 runs tearDown an additional time after all test_* functions
  have been called.
  kward/shunit2#112
@kward
Copy link
Owner

kward commented Oct 24, 2021

Is this still a problem in the HEAD release?

@ypoluektovich
Copy link

Yes.

@Bajger
Copy link

Bajger commented Nov 23, 2021

Yes, tested on release 2.1.8. My output looks like this:

Calling teardown...

Ran 4 tests.

OK
Calling teardown...
PharoLauncherCommonFunctions.sh: line 45: ./pharo-launcher.sh: No such file or directory

@swcline
Copy link

swcline commented Nov 23, 2021 via email

@cezanne
Copy link

cezanne commented Mar 1, 2022

I had the same problem. The following modification resolved my issue.

diff --git a/shunit2 b/shunit2
index 57a45da..ead9de6 100755
--- a/shunit2
+++ b/shunit2
@@ -1367,6 +1367,7 @@ _shunit_execSuite
 if ! oneTimeTearDown; then
   _shunit_fatal "oneTimeTearDown() returned non-zero return code."
 fi
+__shunit_clean=${SHUNIT_TRUE}
 
 # Generate a report summary.
 _shunit_generateReport

cezanne added a commit to cezanne/shunit2 that referenced this issue Mar 1, 2022
Additional oneTimeTearDown() was called at shunit2 exit because there
seemed to be no set for __shunit_clean flag. Fix for kward#112
@larssb
Copy link

larssb commented Aug 6, 2024

Could we please get one of the PR/suggestions to fix this merged into main on kward and get a new release? I just bumped into this and it took some troubleshooting to figure it out.

Would be great to avoid having shunit2 executing the oneTimeTearDown more than exactly once, unless it's a dirty exit.

Here's some of the suggestions I've found:

Thank you very much 👍


N.B. @williamdes are you the "savior" once again 💯 ?

@larssb
Copy link

larssb commented Aug 6, 2024

My workaround is to create a directory in oneTimeSetUp and wrap the tear down code inside a check for that directory inside oneTimeTearDown - second time around, directory is not there, so code is skipped

An alternative to this is to set a global env. var in Bash.

E.g.

oneTimeSetUp()
{
    SOME_SETUP_LOGIC_HERE
    
    export avoidDoubleTearDownExecution="true"
}

....
....

oneTimeTearDown()
{
    if [[ "${avoidDoubleTeadDownExecution}" == "true" ]]; then
        SOME_TEARDOWN_LOGIC_HERE
        
        unset -v avoidDoubleTearDownExecution
    fi
}

The above makes the tear down, in clean cases, where what's in #108 is not needed, only run once.

Enjoy 🥇

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

No branches or pull requests

9 participants