-
Notifications
You must be signed in to change notification settings - Fork 10.3k
scripts/test.sh: Fix unbound variable error when RUN_ARG is empty #21125
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
base: main
Are you sure you want to change the base?
Conversation
When TESTCASE environment variable is not set, RUN_ARG array remains
empty. Under set -o nounset, expanding an empty array "${RUN_ARG[@]}"
causes an "unbound variable" error in bash 3.2+ (especially on macOS).
This fix changes RUN_ARG from an array to a string. When RUN_ARG is
an empty string, $RUN_ARG expands to nothing (no error), and when it
has a value, it expands correctly. This is simpler than adding
conditional checks and maintains strict error checking.
Fixes the issue where `make test` and `make test-unit` fail when
TESTCASE is not set.
Fixes etcd-io#21124
Tested:
- make test-unit (without TESTCASE) - now works
- make test-unit TESTCASE="TestName" - still works
- bash -n scripts/test.sh - syntax validation passes
Signed-off-by: Ixecd <2192629378@qq.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ixecd The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ixecd. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Note: Don't write a essay on something that can be explained in a single sentence. AI here works against you. I guess this is nothing related to what is described in the issue, but to shell difference. Would be good to trace which shells this works or doesn't and whether we want to support them. cc @ivanvc |
Got it, I’ll be careful and get straight to the point whenever I can. You're right, this is indeed a shell version difference issue. The shebang |
|
Hmm, not sure I understood, could you please respond using rhyme. That would make it more understandable for me. :P |
Okay, that’s unexpected -- but alright :) The shebang env with bash so fine, For empty vars, don’t use array, |
|
Hi, @Ixecd. In short, we don't expect compatibility with Bash 3.2. Our scripts use Bash Arrays, and we need at least 4+. If you're using macOS, we recommend installing a newer Bash version with Homebrew. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Fix: Handle empty RUN_ARG array in test.sh to prevent "unbound variable" error
Fixes #21124
Problem
When running
make testormake test-unitwithout setting theTESTCASEenvironment variable, the script fails with:This occurs because:
RUN_ARGarray is initialized as empty whenTESTCASEis not set:RUN_ARG=()set -o nounset(line 55) for strict error checking"${RUN_ARG[@]}"underset -o nounsettriggers an "unbound variable" errorSolution
Changed
RUN_ARGfrom an array to a string. This is the simplest and most elegant solution:RUN_ARG=()toRUN_ARG=""RUN_ARG=("-run=${TESTCASE}")toRUN_ARG="-run=${TESTCASE}""${RUN_ARG[@]}"to$RUN_ARGin all function callsWhen
RUN_ARGis an empty string,$RUN_ARGexpands to nothing (no error), and when it has a value, it expands correctly. This ensures compatibility with bash 3.2+ while maintaining strict error checking withset -o nounset.Changes
Changed
RUN_ARGfrom array to string inscripts/test.sh:RUN_ARG=()toRUN_ARG=""RUN_ARG=("-run=${TESTCASE}")toRUN_ARG="-run=${TESTCASE}"$RUN_ARGinstead of"${RUN_ARG[@]}":unit_passintegration_extraintegration_pass(2 call sites)e2e_pass(2 call sites)robustness_passgrpcproxy_integration_passgrpcproxy_e2e_passTesting
Tested with:
make test-unit(without TESTCASE) - now worksmake test-unit TESTCASE="TestName"- still works as expectedbash -n scripts/test.sh- passesCompatibility
set -o nounsetstrict error checkingTESTCASEremains optionalRelated
make testfails on macOS/bash 3.2TESTCASE, which was the intended behavior based on the script's documentation