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

(FACT-1754) search for matching line with java version #257

Conversation

shuebnersr
Copy link
Contributor

FACT-1754: fact "java_version" fails when JAVA_TOOL_OPTIONS is used

Solution: do not just try to read java version from first line, but search for the matching line from java output.

Note: as in the original code, the fix appears three times - I think it is on purpose to not introduce new functions in this scope.

do not just try to read java version from first line, but search for the matching line from java output
@shuebnersr
Copy link
Contributor Author

CI checks failed, will improve code.

@tphoney
Copy link
Contributor

tphoney commented Sep 21, 2017

Thanks for the PR, keep up the good work @shuebnersr !

mock Facter::Util::Resolution.which('java') for the "other systems" test; before it was not mocked and was based on Java being installed on the test system or not
…when_java_not_installed

Fact1760 test for java version when java not installed
@tphoney
Copy link
Contributor

tphoney commented Sep 25, 2017

There still seems to be issues on the unit tests. getting closer 👍

@shuebnersr
Copy link
Contributor Author

Fixed the unit test for "other systems" (see above), but one of the other two tests is also broken. Still on it, never used Ruby before . :-)

@tphoney
Copy link
Contributor

tphoney commented Sep 26, 2017

Many thanks for your continued persistence @shuebnersr

Use value of variable instead of return statement
Extended unit test for checking handling of JAVA_TOOL_OPTIONS
@shuebnersr
Copy link
Contributor Author

Code improved, CI checks now succeed.

@tphoney tphoney merged commit 5f820fe into puppetlabs:master Oct 2, 2017
@tphoney
Copy link
Contributor

tphoney commented Oct 2, 2017

Great work @shuebnersr !!!

@shuebnersr
Copy link
Contributor Author

You're welcome, @tphoney! And thanks a lot for the support and the great tool!

@Ramesh7 Ramesh7 added the bugfix label Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants