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

add dotted environment variables test #15668

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gdams
Copy link
Contributor

@gdams gdams commented Nov 6, 2023

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

LaurentGoderre
LaurentGoderre previously approved these changes Nov 6, 2023
Comment on lines 55 to 59
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
Copy link
Member

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 😭

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated PTAL

Copy link
Member

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.

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.

4 participants