Repository navigation
Open
Labels
enhancementNew feature or requestNew feature or request
Description
What happens
The tests that pin what the formatter writes call pieces of it, not the routes users run, so two routes could drift apart without a test failing (#219).
| Tests | What they call | What users run |
|---|---|---|
FormatterCasesTest, 915 cases |
Formatter.formatSourceAndFixImports |
the Maven plugin and the Spotless step; the command line, the native binary and the GitHub Action go through FormatFileCallable.formatFile, a second implementation of the same steps |
PartialFormattingTest, 60 tests |
Main --lines without --output-replacements, and Formatter.getFormatReplacements |
IntelliJ IDEA and formatDiff use --output-replacements and FormatterService.getFormatReplacements; before #220 no test ran --output-replacements |
FormatterIntegrationTest, 2 pairs left |
formatSource and then StringWrapper.wrap, put together in the test |
no route |
The cases also never reach the places where the routes differ: all 915 give their expected output with the imports fixed before formatting and after it. The one test that did, ModuleImportTest.keepsACommentBetweenThePartsOfTheModuleName, pinned what formatSourceAndFixImports wrote, which the command line never printed.
And the tests that need a newer JDK are skipped on JDK 21, where a local build runs: 14 skipped there against 2 on JDK 25. The first push of #220 passed locally and failed CI's JDK 25 to 28 builds on that ModuleImportTest.
Proposal
- One implementation.
FormatFileCallable.formatFilecalls the same method asformatSourceAndFixImports, with the flags of the command line, as google-java-format now does with oneFormatter.formatSource(input, ranges, removeUnusedImports, sortImports, reflowLongStrings). Then the 915 cases test the command line. - Cases for the seams. Four cases are ready for
test-cases/imports:commentOfARemovedImportGoesWithTheImportAboveIt,commentOfARemovedOwnPackageImportGoesWithTheImportAboveIt,commentUnderARemovedImportStaysOnTheClassandsemicolonAfterTheLastImportStaysOnALineOfItsOwn. Each passes with Format the same on every route: formatDiff on the JVM, ranges, and the order of fixing imports #220 and fails with the imports fixed first. What a case file cannot say yet, a failure, a JDK it needs, a range, stays inFormatterServiceImplTestandModuleImportTest. - Cases through the routes.
FormatterCasesTestruns each case throughMain, throughFormatterServiceImpl, and in CI's native jobs through the native binary. - Ranges on the cases. For each case, the lines where the code before and after differ are formatted as a range, as
formatDiffdoes, through--output-replacementsand through the in-process service, and the two compared; and formatting any range of the expected output leaves it as it is. - A rule and a task. CLAUDE.md says that a test which pins the output users get goes through
MainorFormatterService, whileImportOrderer,RemoveUnusedImportsandStringWrapperkeep their unit tests. A mise task runs the tests as CI's JDK 25 and 27 builds do, with their TestKit Gradle versions.
Metadata
Metadata
Assignees
Labels
enhancementNew feature or requestNew feature or request
Activity