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

Fix SHELL variable not being properly set in tests #130

Closed
wants to merge 2 commits into from

Conversation

patrick96
Copy link

In commit 3e9578d the shell used in
shunit2_misc_test.sh was changed from ${SHUNIT_SHELL:-sh} to
${SHELL:-sh} because SHUNIT_SHELL was not set anywhere.

However exec ${shell_bin} does not reset the SHELL variable, it
inherits it from the running shell.
This means before that change all misc tests were running with sh and
after the change the running shell was used instead of the shell that is
supposed to be tested.

This can be fixed by manually setting the SHELL variable.

This will now fail the travis build because the most misc tests don't
actually pass on zsh and sh fails testIssue84 (at least on my
machine).

In commit 3e9578d the shell used in
`shunit2_misc_test.sh` was changed from `${SHUNIT_SHELL:-sh}` to
`${SHELL:-sh}` because `SHUNIT_SHELL` was not set anywhere.

However `exec ${shell_bin}` does not reset the `SHELL` variable, it
inherits it from the running shell.
This means before that change all misc tests were running with `sh` and
after the change the running shell was used instead of the shell that is
supposed to be tested.

This can be fixed by manually setting the `SHELL` variable.

This will now fail the travis build because the most misc tests don't
actually pass on `zsh` and `sh` fails `testIssue84` (at least on my
machine).
@kward
Copy link
Owner

kward commented Mar 29, 2020

This looks good so far. Do you have any thoughts on fixing the now failing tests once this change is made?

@patrick96
Copy link
Author

I unfortunately don't have the capacity to look at all of them.

But I did have a look at a few:

testIssue84 was the only one that failed with sh. So my guess was that it involves use of bashisms. Running the underlying test case gives me the following when running with /bin/sh:

--- test_syntax_error
tests/data/testIssue84.sh: line 2: -334: command not found

Ran 1 test.

OK

for /bin/bash the output looks like this:

--- test_syntax_error
tests/data/testIssue84.sh: line 2: ${!#3442}: bad substitution
shunit2:ERROR test_syntax_error() returned non-zero return code.

Ran 1 test.

FAILED (failures=1)

For some reason this produces a failed test under bash but not sh, couldn't tell you why though.

On zsh a bunch of misc tests fail:

testUnboundVariable
ASSERT:assert message was not generated
ASSERT:test count message was not generated
ASSERT:failure message was not generated
shunit2:ERROR testUnboundVariable() returned non-zero return code.
testIssue7
testIssue29
ASSERT:expected:<0> but was:<1>
testIssue69
ASSERT:failure message for assertEquals was not generated
ASSERT:failure message for assertNotEquals was not generated
ASSERT:failure message for assertNull was not generated
ASSERT:failure message for assertNotNull was not generated
ASSERT:failure message for assertSame was not generated
ASSERT:failure message for assertNotSame was not generated
ASSERT:failure message for assertTrue was not generated
ASSERT:failure message for assertFalse was not generated
shunit2:ERROR testIssue69() returned non-zero return code.
testIssue77
ASSERT:failure of oneTimeSetUp() did not end test
ASSERT:failure of setUp() did not end test
ASSERT:failure of tearDown() did not end test
ASSERT:failure of oneTimeTearDown() did not end test
shunit2:ERROR testIssue77() returned non-zero return code.
testIssue84
ASSERT:failure message for syntax error was not generated
shunit2:ERROR testIssue84() returned non-zero return code.
testPrepForSourcing
testEscapeCharInStr
testEscapeCharInStr_specialChars
testExtractTestFunctions
testIssue54
testColors
testColorsWitoutTERM

Ran 13 tests.

FAILED (failures=21)

I have only looked at a few though, but they seem to have the same underlying issue.
testIssue29 and testIssue84 only produce

_shunit_fatal:1: command not found: echo -e

testUnboundVariable produces

_shunit_fatal:1: __shunit_ansi_red: parameter not set

when running with /bin/zsh.

Now zsh definitely supports echo -e and if you run the tests with zsh -c instead of just zsh they work as well (except for test84 which has the same bashism issue).
My guess is that zsh executes a script differently when you call it with exec /bin/zsh -c "${unittestF}" instead of just exec /bin/zsh "${unittestF}".

I have added a commit that extracts out the underlying tests into their own files, this makes it easier to debug because the test scripts are not generated on the fly with sed.
It only does that for the three tests I have analyzed, feel free to discard it, I just found it useful for debugging and maybe you do too.

I wish I could help more, but I really don't have the time right now.

@kward
Copy link
Owner

kward commented Oct 23, 2021

I'm considering this to no longer be a problem as all tests in shunit2_misc_test.sh pass for zsh. If you feel this is in error, please reopen.

@kward kward closed this Oct 23, 2021
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.

2 participants