-
Notifications
You must be signed in to change notification settings - Fork 212
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
shunit2 has not been tested with the proper shell (and the current actual test has been failing) #121
Comments
Actually, $SHELL is set by bash and some shells but not set by dash. So controlled build environment with cleaned environment variable under dash has undefined $SHELL. This patch fixed error for testIssue84 for me for bash build-test started on dash. But dash still fails for dash build-test started on dash. Key difference in set -x log is
DASH is lead by
Also right most value is different. FULL LOGBASH (with set -x):
DASH:
|
I have problem sourcing following script;
By changing first line to use |
Hmmm... even without patch, bash doesn't fail. even with patch, dash fails. Maybe I am getting confused. I decided to test shunit2_misc_test.sh only for bash for Debian packaging for now. All others are tested for 5 shells: /bin/bash /bin/dash /bin/ksh /bin/mksh /bin/zsh Is it OK to release 2.1.8pre to Debian unstable repository? Debian has 2.1.6 variant now. Thanks for your nice package. |
Now I see what was happening. The strange ${SHELL-sh} in shunit2_misc_test.sh is almost meant to use bash no matter what shell test_runner requests. That means cheating on testing code for each shell. That is not right thing to do. When this is invoked from interactive bash shell $SHELL is already set as bash and bash is used. But it it is started from other shell like:
Things get bad nowadays. This cheating worked mostly on old Ubuntu/Debian where /bin/sh was linked to bash. No more. /bin/sh is linked to dash now. So if you still want to cheat like this, ${SHELL-bash} needs to be here. But that cheating fails on ksh since:
So if we really have to cheat, use "bash" there. I think this cheating seems to beat the purpose of having these test scripts. dash:
ksh:
mksh, zsh -- looks same as dash since it uses dash by cheating under modern Debian. (If I set SHELL to something, more errors come out from non-bash shells.) I think we need to make test case for dash at least. |
On Ubuntu
On Alpine
For the colors error
|
Could you let me know if these are still broken on HEAD? |
It's still broken. I am using zsh as my interactive shell on macOS. In other words,
|
Additional test results. SHELL=/bin/sh ./test_runner -s /bin/sh [OK]
SHELL=/bin/bash ./test_runner -s /bin/bash [OK]
SHELL=/bin/dash ./test_runner -s /bin/dash [FAILED]
SHELL=/bin/ksh ./test_runner -s /bin/ksh [FAILED]
SHELL=/bin/zsh ./test_runner -s /bin/zsh [FAILED]
SHELL=/usr/local/bin/mksh ./test_runner -s /usr/local/bin/mksh [FAILED]
|
Additional oneTimeTearDown() was called at shunit2 exit because there seemed to be no set for __shunit_clean flag. Fix for kward#121
Can someone help me fixing this error ? On my workstation tests pass but not on a CI: https://salsa.debian.org/debian/shunit2/-/jobs/2774040#L1015
PS: cezanne@e736e3e does not work |
shunit2_misc_test.sh
is tested with the shell that set in the environment variable$SHELL
.e.g.
However
$SHELL
is not the current running shell, but the shell when login.test_runner
tries to test all installed shells, but it test only login shell (probably bash only).I did a quick hack to test, then the current (ba130d6 and v2.1.7) test failed.
(note:
$SHUNIT_SHELL
was used instead of$SHELL
on v2.1.7)Result of ./test_runner
The text was updated successfully, but these errors were encountered: