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

Fix: Support JVM version string from old (< v8.9) and new (>= v8.9) gradle version #683

Merged
merged 9 commits into from
Nov 15, 2024
Merged

Fix: Support JVM version string from old (< v8.9) and new (>= v8.9) gradle version #683

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a227030
fix(validator): support version string from old and newest gradle
jeffryang24 Sep 5, 2024
bf105be
fix(test): fix unit test
jeffryang24 Sep 5, 2024
233ea52
test: add more version and runtime to the test
jeffryang24 Sep 5, 2024
50e35ac
refactor(validator): use full label instead
jeffryang24 Sep 8, 2024
2e24e33
fix(validator): use in operator to check JVM substring
jeffryang24 Sep 12, 2024
5f49967
Merge branch 'develop' into fix/support-old-and-new-gradle-jvm-versio…
jeffryang24 Oct 18, 2024
52e30e3
Merge branch 'develop' into fix/support-old-and-new-gradle-jvm-versio…
jeffryang24 Oct 23, 2024
59c50a2
Merge branch 'develop' into fix/support-old-and-new-gradle-jvm-versio…
jeffryang24 Nov 10, 2024
e79fe74
Merge branch 'develop' into fix/support-old-and-new-gradle-jvm-versio…
mndeveci Nov 15, 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
2 changes: 1 addition & 1 deletion aws_lambda_builders/workflows/java_gradle/gradle_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,5 +83,5 @@ def _get_jvm_string(self, gradle_path):

for line in stdout.splitlines():
l_dec = decode(line)
if l_dec.startswith("JVM"):
if l_dec.startswith("JVM") or l_dec.startswith("Launcher JVM"):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This could be simplified to l_dec.contains("JVM")

Copy link
Contributor Author

@jeffryang24 jeffryang24 Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I see, I will change it with .contains(...) alike method, probably use x in y style. 👍

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 with in operator to check the JVM substring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @hnnasit , is there something that I need to do so this PR can be merged? Thank you 🙇

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @jeffryang24, our CI checks are failing consistently (and randomly out of the blue). The failures are unrelated to your changes, but we're working on fixing them. At the moment, there is no action needed from your side. Thanks for waiting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, noted 👍

return l_dec
34 changes: 34 additions & 0 deletions tests/unit/workflows/java_gradle/test_gradle_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,40 @@ def test_emits_warning_when_version_string_not_found(self):
validator.validate(runtime_path=self.runtime_path)
self.mock_log.warning.assert_called_with(GradleValidator.VERSION_STRING_WARNING, self.runtime_path)

@parameterized.expand(
[
("1.8.0", "java8"),
("11.0.0", "java11"),
("17.0.0", "java17"),
("21.0.0", "java21"),
]
)
def test_does_not_emit_warning_for_version_string_in_gradle_lt_8_9(self, version, runtime):
version_string = f"JVM: {version}".encode()
self.mock_os_utils.popen.side_effect = [FakePopen(stdout=version_string, returncode=0)]
validator = GradleValidator(
runtime=runtime, architecture=self.architecture, os_utils=self.mock_os_utils, log=self.mock_log
)
validator.validate(runtime_path=self.runtime_path)
self.mock_log.warning.assert_not_called()

@parameterized.expand(
[
("1.8.0", "java8"),
("11.0.0", "java11"),
("17.0.0", "java17"),
("21.0.0", "java21"),
]
)
def test_does_not_emit_warning_for_version_string_in_gradle_ge_8_9(self, version, runtime):
version_string = f"Launcher JVM: {version}".encode()
self.mock_os_utils.popen.side_effect = [FakePopen(stdout=version_string, returncode=0)]
validator = GradleValidator(
runtime=runtime, architecture=self.architecture, os_utils=self.mock_os_utils, log=self.mock_log
)
validator.validate(runtime_path=self.runtime_path)
self.mock_log.warning.assert_not_called()

@parameterized.expand(
[
("11.0.0", "java11"),
Expand Down
Loading