-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
add dotted environment variables test #15668
base: master
Are you sure you want to change the base?
Conversation
test/tests/run-java-in-container.sh
Outdated
if [ $testDir == *"dotted-environment-variables"* ]; then | ||
docker run --rm -e "variable.with.a.dot=a.dotted.value" "$newImage" java -cp . container | ||
else | ||
docker run --rm "$newImage" java -cp . container | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one-test-specific conditional feels a bit icky in an otherwise generic script 😞
Our test framework wasn't really built with this kind of thing in mind, and it's really showing its age 😭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does pattern matching work when ==
is used with [ ]
? I've always read the bash(1) man page as saying that that pattern matching was employed with [[ ]]
.
[[ expression ]]
…
When the == and != operators are used, the string to the right of the operator is considered a pattern and matched according to the rules described below under Pattern Matching, as if the extglob shell option were enabled.
Rereading, the comma makes it a bit ambiguous as to whether it is saying the ==
enables Pattern Matching at all or Pattern Matching with extglob enabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the double square brackets are needed:
$ bash --version
GNU bash, version 5.2.15(1)-release (aarch64-apple-darwin22.1.0)
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
$ bash -c 'testDir=a-dotted-environment-variables-dir; if [ $testDir == *"dotted-environment-variables"* ]; then echo true; else echo false; fi;'
false
$ bash -c 'testDir=a-dotted-environment-variables-dir; if [[ $testDir == *"dotted-environment-variables"* ]]; then echo true; else echo false; fi;'
true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated PTAL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one-test-specific conditional feels a bit icky in an otherwise generic script 😞
Our test framework wasn't really built with this kind of thing in mind, and it's really showing its age 😭
This feedback is still unresolved -- I'm not sure what to suggest, however.
This should help us catch bugs like adoptium/containers#415 in the future.
This could be made an OpenJDK-specific test but I figured there is likely no harm running it for all images