Add support for Palantir Java Formatter

[eirnym]

trafficstars

Palantir Java Formatter has a bit different settings than Google Java Format.

As the original project doesn't provide standalone version, repackager CI has been used.

[eirnym]

Palantir Java formatter currently requires Java Version < 21 and is not available if java version is above 21, so coverage won't show 100% if it's the case.

[macisamuele]

If this is the case we need to make sure that

1. at runtime we check the jdk in use and bail out if not on the proper version (there is already an helper for that)
2. have unit test verifying that
3. have tests that run the tool end to end with pytest.skipif tk make sure that the test is only run if new sdk available

Once the points above are verified tests will be green and we could merge this

[eirnym]

I think I have this test, I'll double check it.

[eirnym]

Anyway this won't help with coverage that you'd like to be 100%

[eirnym]

Do you mind if I'll split tests for google formatter and palantir into separate files?

[macisamuele]

Do you mind if I'll split tests for google formatter and palantir into separate files?

I would rather prefer to keep the tests of a single module into a single test file, it makes it managing them more predictable.

Test coverage can be kept high, it is sufficient to check what the test is mentioning as not covered lines

  py310: commands[2]> coverage report
  Name                                                         Stmts   Miss  Cover   Missing
  ------------------------------------------------------------------------------------------
  language_formatters_pre_commit_hooks/pretty_format_java.py      54      1    98%   220
  ------------------------------------------------------------------------------------------
  TOTAL                                                          356      1    99%

Checking the code I see that the following line is not covered https://github.com/macisamuele/language-formatters-pre-commit-hooks/blob/881f4e1e7ed0641225d1a5c9267da927d3fe97f7/language_formatters_pre_commit_hooks/pretty_format_java.py#L220

The coverage missing is due to the fact that we cannot guarantee that end-to-end test is run with Java 21 in all the environments, hence let's add # pragma: nocover for now to silence the coverage report and we can check it on a second round.

PS: I would suggest to rebase the branch on the latest master branch such that you'll have tests run with Java 21 as well (#219)

[eirnym]

@macisamuele I've done as you said.

• rebased to master
• added nocover for replace flag
• Fixed one test name
• passed JAVA_HOME to tox (as on a local computer many Java version are installed)

For java pre 21, coverage is 100%, while on java 21 palantir coverage won't pass because of test precondition.

[eirnym]

@macisamuele any news?

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (2f03d55) to head (b879caa). Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #213   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          355       371   +16     
=========================================
+ Hits           355       371   +16     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[eirnym]

@macisamuele you can't guarantee full coverage on Java 21, as tool doesn't support Java 21 at all, as it just don't run because of internal dependencies and slow development process (due to licensing). I can comment all lines for Palantir code as # pragma: nocover if you see this would be a solution

[CopperGiraffe]

Hello there, Palantir support would be great, is there any news on the subject?

[eirnym]

@macisamuele Palantir fixed the issue and now supports Java 21. I updated version, rebased and added tests. Could you please review it again?

[macisamuele]

Please address coverage and then ready to merge.

language_formatters_pre_commit_hooks/pretty_format_java.py      59      3    95%   233-239

[eirnym]

@macisamuele Finally nailed it!

[eirnym]

Any news?

[macisamuele]

Had a very busy time in the last days. Enabled tests to run and if they pass this PR will be merged. Tomorrow I'll check the status of this and release a new version if the change was merged

[macisamuele]

Change is merged , at the same time pre-commit were not really fixed. Going to fix them directly on master and take care of the release

[eirnym]

Thank you!