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.

Sign up for GitHub

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

Jump to bottom

add dotted environment variables test #15668

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
+19 −1
Draft

add dotted environment variables test #15668

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec3b541
add dotted environment variables test
gdams Nov 6, 2023
561aff1
Update run-java-in-container.sh
gdams Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions test/tests/dotted-environment-variables/container.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
public class container {
/**
* Check if dotted env vars are supported.
*/
public static void main(String[] args) {
// get value of variable.with.a.dot and print it out
String value = System.getenv("variable.with.a.dot");
System.out.println(value);
System.exit(0);
}
}
1 change: 1 addition & 0 deletions test/tests/dotted-environment-variables/expected-std-out.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a.dotted.value
1 change: 1 addition & 0 deletions test/tests/dotted-environment-variables/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../run-java-in-container.sh
7 changes: 6 additions & 1 deletion test/tests/run-java-in-container.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,9 @@ COPY --from=jdk /container /container
WORKDIR /container
EOD

docker run --rm "$newImage" java -cp . container
# if testDir contains dotted-environment-variables we need to add an environment variable to the docker run command
if [ $testDir == *"dotted-environment-variables"* ]; then
gdams marked this conversation as resolved.
Show resolved Hide resolved
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.