feat(cli): add Java support and update related tests#13
Open
lcmetzger wants to merge 2 commits intocompozy:mainfrom
Open
feat(cli): add Java support and update related tests#13lcmetzger wants to merge 2 commits intocompozy:mainfrom
lcmetzger wants to merge 2 commits intocompozy:mainfrom
Conversation
- Add Java adapter support to parsing, scanning, and generation flow. - Expand unit and integration tests across adapters, CLI, lint, and generate paths. - Add Java ingest workflow artifacts (PRD, TechSpec, ADRs, tasks, and rollout docs). Made-with: Cursor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 8 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
WalkthroughThis PR adds comprehensive Java language support across the codebase, including a tree-sitter-based JavaAdapter for parsing Java source files, semantic and syntactic resolution for imports and method calls with fallback mechanisms, Java diagnostics governance for parse errors and fallback warnings, CLI flags, benchmark policies, and vault rendering for Java diagnostic summaries. Changes
Sequence DiagramsequenceDiagram
actor CLI
participant JavaAdapter
participant TreeSitter
participant ModuleMetadataLoader
participant DeepResolver
participant SyntacticResolver
participant DiagnosticsBuilder
CLI->>JavaAdapter: ParseFiles(files, rootPath)
JavaAdapter->>TreeSitter: Parse Java source file
TreeSitter-->>JavaAdapter: Tree + symbols (class, method, import)
JavaAdapter->>ModuleMetadataLoader: Discover modules from Gradle/Maven
ModuleMetadataLoader-->>JavaAdapter: Module dependency map
JavaAdapter->>DeepResolver: Resolve imports & calls (Phase 1: Semantic)
DeepResolver->>DeepResolver: Lookup FQN in global context
DeepResolver-->>JavaAdapter: Resolved relations + failures
JavaAdapter->>SyntacticResolver: Resolve failures (Phase 2: Syntactic)
SyntacticResolver->>SyntacticResolver: Fallback pattern matching
SyntacticResolver-->>JavaAdapter: Additional relations + remaining unresolved
JavaAdapter->>DiagnosticsBuilder: Build diagnostics from unresolved & metadata
DiagnosticsBuilder-->>JavaAdapter: Warnings & errors (truncated if needed)
JavaAdapter-->>CLI: ParsedFile{nodes, relations, diagnostics}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (10)
internal/generate/benchmark_policy_test.go (1)
92-95: Prefert.TempDir()over hardcoded/tmpin tests.Using
t.TempDir()keeps the test environment isolated and portable.Suggested change
- options := benchmarkGenerateOptions("/tmp/canonical-repo") - if options.RootPath != "/tmp/canonical-repo" { - t.Fatalf("RootPath = %q, want /tmp/canonical-repo", options.RootPath) - } + rootPath := t.TempDir() + options := benchmarkGenerateOptions(rootPath) + if options.RootPath != rootPath { + t.Fatalf("RootPath = %q, want %q", options.RootPath, rootPath) + }As per coding guidelines,
**/*_test.go: "Default to table-driven tests with focused helpers and t.TempDir() for filesystem isolation".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/generate/benchmark_policy_test.go` around lines 92 - 95, Replace the hardcoded "/tmp/canonical-repo" test input with a t.TempDir()-based path: call tmp := t.TempDir(), build the repo path with filepath.Join(tmp, "canonical-repo"), pass that into benchmarkGenerateOptions and assert options.RootPath equals that joined path; also ensure the test imports "path/filepath" if not already present and remove the hardcoded "/tmp" usage.internal/cli/ingest_test.go (1)
443-568: Consolidate duplicated contract tests into a single table-driven subtest suite.Line 443 and Line 506 repeat almost the same setup/assertions. This is a good candidate for one table-driven test with
t.Run("Should ...")cases fordryRun=true/false.♻️ Suggested refactor
-func TestIngestCodebaseCommandJSONContractRequiredKeysFullRun(t *testing.T) { - // ... -} - -func TestIngestCodebaseCommandJSONContractRequiredKeysDryRun(t *testing.T) { - // ... -} +func TestIngestCodebaseCommandJSONContractRequiredKeys(t *testing.T) { + testCases := []struct { + name string + dryRun bool + }{ + {name: "Should validate codebase ingest JSON contract for full run", dryRun: false}, + {name: "Should validate codebase ingest JSON contract for dry run", dryRun: true}, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.name, func(t *testing.T) { + restoreIngestGlobals(t) + // shared stubs... + // args include --dry-run only when testCase.dryRun + // decode + shape + semantics assertions using testCase.dryRun + }) + } +}As per coding guidelines,
**/*_test.go:Default to table-driven tests with focused helpers and t.TempDir()andMUST use t.Run("Should...") pattern for ALL test cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/ingest_test.go` around lines 443 - 568, The two tests TestIngestCodebaseCommandJSONContractRequiredKeysFullRun and TestIngestCodebaseCommandJSONContractRequiredKeysDryRun duplicate setup and assertions; convert them into a single table-driven test that iterates over cases for dryRun true/false using t.Run("Should ...") subtests, use t.TempDir() instead of hard-coded /tmp paths, and factor common setup (overriding runIngestTopicInfo and runGenerate) into a helper or closure; keep calls to newRootCommand(), decodeJSONMap(), assertCodebaseIngestContractShape(), and assertCodebaseIngestContractSemantics() but call them per-case with the appropriate dryRun flag so runGenerate returns the correct GenerationSummary for each case.internal/generate/generate.go (1)
69-77: Centralize default adapter registration to avoid drift.The default adapter slice is duplicated in both constructors. Extracting a single helper for default adapters will prevent future mismatch bugs when adding/removing languages.
♻️ Suggested refactor
+func defaultLanguageAdapters() []models.LanguageAdapter { + return []models.LanguageAdapter{ + adapter.TSAdapter{}, + adapter.GoAdapter{}, + adapter.RustAdapter{}, + adapter.JavaAdapter{}, + } +} + func newRunner() runner { return runner{ scanWorkspace: scanner.ScanWorkspace, - adapters: []models.LanguageAdapter{ - adapter.TSAdapter{}, - adapter.GoAdapter{}, - adapter.RustAdapter{}, - adapter.JavaAdapter{}, - }, + adapters: defaultLanguageAdapters(), // ... } } func (r runner) withDefaults() runner { if len(r.adapters) == 0 { - r.adapters = []models.LanguageAdapter{ - adapter.TSAdapter{}, - adapter.GoAdapter{}, - adapter.RustAdapter{}, - adapter.JavaAdapter{}, - } + r.adapters = defaultLanguageAdapters() }Also applies to: 373-379
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/generate/generate.go` around lines 69 - 77, Extract the duplicated default adapters slice into a single helper function (e.g., defaultLanguageAdapters) that returns []models.LanguageAdapter and replace the inline literal in newRunner (which sets runner.adapters) and the other constructor that currently duplicates the slice (the one around lines ~373-379) to call this helper; update the runner struct initialization to use defaultLanguageAdapters() so all constructors share the same source of truth and prevent drift when adding/removing adapters.internal/lint/lint_test.go (1)
197-325: Refactor Java governance tests into a single table-drivent.Run("Should...")suite.These four tests validate adjacent policy permutations and can share setup/assertions with per-case expectations to reduce drift and make future governance changes easier to maintain.
As per coding guidelines,
**/*_test.go:Default to table-driven tests with focused helpers and t.TempDir()andMUST use t.Run("Should...") pattern for ALL test cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/lint/lint_test.go` around lines 197 - 325, Combine the four separate tests (TestLintJavaDiagnosticsGovernanceParseErrorsBlockByDefault, TestLintJavaDiagnosticsGovernanceFallbackDisabledByDefault, TestLintJavaDiagnosticsGovernanceAppliesCustomFallbackThreshold, TestLintJavaDiagnosticsGovernanceMessageUsesMachineReadableCounts) into a single table-driven test that uses t.Run("Should ...") subtests and t.TempDir(), extracting common setup (newTestTopic/writeMarkdownFile/mustLint or LintWithOptions) into a helper; create a slice of cases with fields for input metadata, optional LintOptions, expected presence/absence of models.LintIssue (and expected Target/Severity/payload assertions for the machine-readable message case), iterate cases and for each call t.Run(case.name, func(t *testing.T){ ... }) performing the shared setup/assertions and only per-case checks in the loop. Ensure you preserve the unique expectations for JAVA_PARSE_ERROR, JAVA_RESOLUTION_FALLBACK and the JSON message assertion (unmarshal parseIssue.Message) within the appropriate case.internal/cli/java_portfolio_playbook_test.go (1)
23-90: Convert repeated playbook fragment checks into one table-driven subtest.The three tests are structurally identical (load file + assert fragments). A single table-driven test with
t.Run("Should ...")cases will keep this easier to extend and less repetitive.As per coding guidelines,
**/*_test.go:Default to table-driven tests with focused helpers and t.TempDir()andMUST use t.Run("Should...") pattern for ALL test cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/java_portfolio_playbook_test.go` around lines 23 - 90, Three near-identical tests (TestJavaPortfolioPlaybookIncludesGovernanceAndContractRequirements, TestJavaPortfolioPlaybookIncludesFallbackAndUnresolvedGuidance, TestJavaPortfolioPlaybookCommandsAlignWithCurrentCLI) should be consolidated into one table-driven test that uses t.Run("Should ...") subtests; replace the three separate functions with a single TestJavaPortfolioPlaybookFragments that calls readJavaPortfolioPlaybook once and iterates over a slice of cases where each case has a name and requiredFragments slice, running each case as a subtest (call t.Run(caseName, func(t *testing.T) { t.Parallel(); ... })) and performing the strings.Contains checks inside the subtest; keep the helper readJavaPortfolioPlaybook, preserve t.Parallel semantics at the subtest level, and follow the t.Run naming convention and table-driven pattern as required.internal/generate/generate_integration_test.go (1)
179-260: Convert the new integration cases toShould...subtests.The new Java coverage is useful, but these additions still expand the file with standalone cases instead of grouped
t.Run("Should...")scenarios.As per coding guidelines,
**/*_test.go: Default to table-driven tests with focused helpers and t.TempDir() for filesystem isolationandMUST use t.Run("Should...") pattern for ALL test cases`.Also applies to: 302-391
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/generate/generate_integration_test.go` around lines 179 - 260, The two new standalone tests TestGenerateIntegrationDryRunSelectsJavaAdapterForMixedWorkspace and TestGenerateIntegrationBuildsVaultFromJavaPhase2Workspace should be converted into t.Run("Should...") subtests (and folded into a table-driven pattern where appropriate) per test guidelines: wrap each scenario body in t.Run with descriptive "Should..." names, move repeated setup into focused helpers (reuse writeFixtureFile/writeJavaPhase2Fixture and newRunner), keep using t.TempDir for isolation, and update any similar cases in the same file (around the 302-391 additions) to follow the same t.Run table-driven structure so all cases are subtests rather than standalone Test* functions.internal/generate/generate_test.go (1)
213-283: Group the new cases into table-drivent.Run("Should...")suites.These Java adapter/telemetry additions are mostly standalone tests with duplicated setup and assertions, which makes the file harder to extend as more languages or telemetry fields are added.
As per coding guidelines,
**/*_test.go: Default to table-driven tests with focused helpers and t.TempDir() for filesystem isolationandMUST use t.Run("Should...") pattern for ALL test cases`.Also applies to: 733-925
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/generate/generate_test.go` around lines 213 - 283, The tests TestSelectAdaptersIncludesJavaWhenWorkspaceHasJava, TestSelectAdaptersForMixedWorkspace, TestNewRunnerRegistersJavaAdapterInExpectedOrder, and TestRunnerWithDefaultsIncludesJavaAdapterWhenAdaptersUnset should be converted into table-driven t.Run suites to remove duplication: create a single test table (cases with name, input languages, adapters, expected names/count/order) and iterate with t.Run for the selectAdapters cases and another t.Run table for runner/withDefaults adapter-order assertions; extract shared setup into small helpers (e.g., makeFakeAdapters or adapterNames helper is fine) and use t.TempDir() where filesystem isolation is needed, replacing repeated setup code, ensuring each case uses t.Parallel() inside its t.Run body if appropriate.internal/adapter/java_adapter_integration_test.go (1)
13-628: Keep the integration coverage, but organize it asShould...subtests.These scenarios are meaningful integration cases, but they're still added as many one-off top-level tests rather than grouped subtests around the same workflow.
As per coding guidelines,
**/*_test.go: Default to table-driven tests with focused helpers and t.TempDir() for filesystem isolation,MUST use t.Run("Should...") pattern for ALL test cases, and**/*_integration_test.go: Integration tests use the integration build tag and live next to the packages they exercise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/adapter/java_adapter_integration_test.go` around lines 13 - 628, The many top-level tests (e.g. TestJavaAdapterBuildsCrossFileImportAndCallRelations, TestJavaAdapterBuildsWildcardImportRelationsAcrossFiles, TestJavaAdapterOutputIsDeterministicAcrossRuns, TestJavaAdapterPartialMetadataFallsBackWithoutFailingIngest, TestJavaAdapterMissingWildcardPackageFallsBackWithoutFailingIngest, TestJavaAdapterResolvesNestedAndTopLevelRelationsAcrossFiles, TestJavaAdapterAmbiguousImportsRemainStableAcrossRuns, TestJavaAdapterModuleHintsImproveAmbiguousImportResolution, TestJavaAdapterWithoutModuleHintsKeepsAmbiguousFallbackPath, TestJavaAdapterPhase2EnterpriseScenarioRegression) should be reorganized into t.Run subtests using the "Should..." naming convention and converted to a table-driven structure where possible; add an integration build tag at the top of the file, wrap each scenario with t.Run("Should ...", func(t *testing.T){ t.Parallel(); ... }), migrate any filesystem usage to t.TempDir() and update helper calls (parseJavaSources, parseJavaSourcesWithRepositoryFiles) to accept/use the temp dir or ensure isolation via helpers, and consolidate shared setup/teardown into focused helper functions so each subtest is terse and follows the table-driven/testing guidelines.internal/cli/workflow_test_helpers_test.go (1)
324-508: Align the helper tests with the repo's required subtest pattern.The new helper coverage is fine, but these cases still rely on top-level one-offs and
t.Run(testCase.name, ...)names that don't follow the requiredShould...convention.As per coding guidelines,
**/*_test.go: Default to table-driven tests with focused helpers and t.TempDir() for filesystem isolationandMUST use t.Run("Should...") pattern for ALL test cases`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/workflow_test_helpers_test.go` around lines 324 - 508, Update the tests to use the required "Should..." subtest naming pattern: in TestValidateJavaCodebaseSummary change the existing t.Run(testCase.name, ...) to t.Run("Should "+testCase.name, ...) so every table-driven case is named "Should ..."; also wrap the single-case TestAssertJavaCodebaseSummaryPassesForValidInput body inside a t.Run("Should pass for valid input", func(t *testing.T){ ... }) to follow the MUST use t.Run("Should...") convention; keep existing use of t.Parallel() and t.TempDir().internal/adapter/java_adapter_test.go (1)
15-1389: Restructure this suite into groupedt.Run("Should...")tables.The coverage is valuable, but dozens of top-level tests for closely related resolver branches make the suite harder to scan and maintain than it needs to be.
As per coding guidelines,
**/*_test.go: Default to table-driven tests with focused helpers and t.TempDir() for filesystem isolationandMUST use t.Run("Should...") pattern for ALL test cases`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/adapter/java_adapter_test.go` around lines 15 - 1389, Group the many related branch checks into table-driven subtests using t.Run for each logical suite (e.g., consolidate TestResolveJavaDeepImport, TestResolveJavaDeepCallTargetBranches, TestResolveJavaCallTargetBranches, TestResolveJavaSyntacticImport and their subcases) by creating a slice of test cases with name, inputs, expected outputs and iterating over them calling t.Run(tc.name, func(t *testing.T) { ... }); keep the existing helper calls to resolveJavaDeepImport, resolveJavaDeepCallTarget, resolveJavaCallTarget, resolveJavaSyntacticImport and assertions but move per-branch setup into each table entry, reuse t.TempDir()/parse helpers as needed, and ensure deterministic ordering by sorting case keys if necessary so the semantics of existing assertions remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/adapter/java_adapter.go`:
- Around line 1796-1806: The artifactId extraction in parseMavenModulePomSignals
incorrectly returns the first <artifactId> (often the parent); update
parseMavenModulePomSignals to scope the lookup to the module's own artifactId by
ignoring any <artifactId> inside a <parent> block (or by extracting the
<artifactId> that is a direct child of <project>). Concretely: before applying
javaMavenArtifactPattern, strip or skip the substring corresponding to the
<parent>...</parent> block (or use a non-greedy regex that matches
<project.*?>.*?<artifactId>...</artifactId> at the project level) so artifactID
reflects the module's own <artifactId> (keep references to
parseMavenModulePomSignals and javaMavenArtifactPattern).
In `@internal/adapter/treesitter_test.go`:
- Around line 37-40: Test case names for the Java subtests do not follow the
repository convention requiring t.Run("Should...") naming; update the test
entries that currently use name: "java" (and the other Java entries around lines
99-104) so that their t.Run invocations use descriptive "Should..." names (e.g.,
"Should parse Java constructs") when calling the subtests, and keep the same
load: func() *tree_sitter.Language { return javaLanguage() } and other fields
unchanged; ensure the t.Run string follows the exact "Should..." prefix for all
Java-related subtests.
In `@internal/cli/generate_test.go`:
- Around line 230-233: The test currently checks for the raw substring "java"
which can yield false positives; update the assertion in generate_test.go so it
looks for an exact token match instead (e.g. use a word-boundary regex like
`\bjava\b` or check for newline/delimiter-wrapped token such as "\njava\n") when
inspecting stdout.String() instead of the plain "java" fragment, keeping the
other fragments (supportedCodebaseLanguagesHelp(), "--dry-run") unchanged.
In `@internal/cli/java_portfolio_playbook_integration_test.go`:
- Around line 13-96: The test
TestCLIIntegrationJavaPortfolioPlaybookCommandsAndSemantics duplicates existing
Java contract/telemetry/lint assertions; remove or narrow the redundant checks
(the dry-run/full-run contract asserts via
assertCodebaseIngestContractShape/assertCodebaseIngestContractSemantics, the
parse telemetry checks using findJSONStageCompletedEvent/eventFieldInt, and the
lint assertion) and instead keep only playbook-specific behavior or delegate to
the shared workflow integration path; update the test to call the shared helper
(or remove the duplicate blocks around runCLIJSON/runCLIWithStreams, the
json.Unmarshal into codebaseIngestResult, assertJavaCodebaseSummary,
parseCompletedDryRun/parseCompletedFullRun checks, and the final lint run) so
this file only verifies playbook-specific commands and semantics.
In `@internal/generate/benchmark_policy_test.go`:
- Around line 9-114: Rename the subtests and wrap single-case tests to follow
the "Should..." t.Run pattern: in TestMedianDurationFromSamples change each
subtest Run name to "Should return middle for odd sample count", "Should return
midpoint average for even sample count", and "Should fail on empty samples"
(keep using medianDurationFromSamples); for the other top-level tests
(TestCanonicalJavaBenchmarkFixtures, TestBenchmarkGenerateOptions,
TestCanonicalJavaBenchmarkPolicy) wrap their assertion blocks inside
t.Run("Should ...") subtests (e.g., TestCanonicalJavaBenchmarkFixtures ->
t.Run("Should return canonical Java benchmark fixtures") checking
canonicalJavaBenchmarkFixtures(), TestBenchmarkGenerateOptions -> t.Run("Should
build benchmark generate options") checking benchmarkGenerateOptions(), and
TestCanonicalJavaBenchmarkPolicy -> t.Run("Should return canonical Java
benchmark policy") checking canonicalJavaBenchmarkPolicy()), preserving
t.Parallel() where present and all existing assertions.
In `@internal/generate/generate_integration_test.go`:
- Around line 262-300: The integration test
TestGenerateIntegrationJavaIngestPerformanceBudget currently enforces a
wall-clock overhead budget that can flake on shared CI; make the budget
assertion opt-in by guarding the fail path with an environment flag or testing
condition (e.g., check os.Getenv("ENABLE_PERF_BUDGET") == "1" or skip when
testing.Short() is true) so the test still measures durations and logs the
overhead but only calls t.Fatalf when the opt-in flag is set; update the
function (and references to policy/overhead logic) so it always computes and
t.Logf the results but only enforces policy.OverheadBudget when the opt-in
condition is met.
In `@internal/scanner/scanner_test.go`:
- Around line 248-257: The subtest names in the table-driven tests (the slice
with entries like name: "go", "rust", etc.) must follow the t.Run("Should...")
naming convention; update each test case's name field (and the corresponding
t.Run invocation) to a descriptive "Should ..." phrase (e.g., "Should detect Go
files", "Should detect Rust files", "Should return unsupported for dts", "Should
return unsupported for README") so all t.Run calls use the required pattern;
apply the same renaming to the additional table entries referenced around the
other block (the cases at lines ~261-273) to keep names consistent across
scanner_test.go and ensure assertions remain unchanged.
---
Nitpick comments:
In `@internal/adapter/java_adapter_integration_test.go`:
- Around line 13-628: The many top-level tests (e.g.
TestJavaAdapterBuildsCrossFileImportAndCallRelations,
TestJavaAdapterBuildsWildcardImportRelationsAcrossFiles,
TestJavaAdapterOutputIsDeterministicAcrossRuns,
TestJavaAdapterPartialMetadataFallsBackWithoutFailingIngest,
TestJavaAdapterMissingWildcardPackageFallsBackWithoutFailingIngest,
TestJavaAdapterResolvesNestedAndTopLevelRelationsAcrossFiles,
TestJavaAdapterAmbiguousImportsRemainStableAcrossRuns,
TestJavaAdapterModuleHintsImproveAmbiguousImportResolution,
TestJavaAdapterWithoutModuleHintsKeepsAmbiguousFallbackPath,
TestJavaAdapterPhase2EnterpriseScenarioRegression) should be reorganized into
t.Run subtests using the "Should..." naming convention and converted to a
table-driven structure where possible; add an integration build tag at the top
of the file, wrap each scenario with t.Run("Should ...", func(t *testing.T){
t.Parallel(); ... }), migrate any filesystem usage to t.TempDir() and update
helper calls (parseJavaSources, parseJavaSourcesWithRepositoryFiles) to
accept/use the temp dir or ensure isolation via helpers, and consolidate shared
setup/teardown into focused helper functions so each subtest is terse and
follows the table-driven/testing guidelines.
In `@internal/adapter/java_adapter_test.go`:
- Around line 15-1389: Group the many related branch checks into table-driven
subtests using t.Run for each logical suite (e.g., consolidate
TestResolveJavaDeepImport, TestResolveJavaDeepCallTargetBranches,
TestResolveJavaCallTargetBranches, TestResolveJavaSyntacticImport and their
subcases) by creating a slice of test cases with name, inputs, expected outputs
and iterating over them calling t.Run(tc.name, func(t *testing.T) { ... }); keep
the existing helper calls to resolveJavaDeepImport, resolveJavaDeepCallTarget,
resolveJavaCallTarget, resolveJavaSyntacticImport and assertions but move
per-branch setup into each table entry, reuse t.TempDir()/parse helpers as
needed, and ensure deterministic ordering by sorting case keys if necessary so
the semantics of existing assertions remain identical.
In `@internal/cli/ingest_test.go`:
- Around line 443-568: The two tests
TestIngestCodebaseCommandJSONContractRequiredKeysFullRun and
TestIngestCodebaseCommandJSONContractRequiredKeysDryRun duplicate setup and
assertions; convert them into a single table-driven test that iterates over
cases for dryRun true/false using t.Run("Should ...") subtests, use t.TempDir()
instead of hard-coded /tmp paths, and factor common setup (overriding
runIngestTopicInfo and runGenerate) into a helper or closure; keep calls to
newRootCommand(), decodeJSONMap(), assertCodebaseIngestContractShape(), and
assertCodebaseIngestContractSemantics() but call them per-case with the
appropriate dryRun flag so runGenerate returns the correct GenerationSummary for
each case.
In `@internal/cli/java_portfolio_playbook_test.go`:
- Around line 23-90: Three near-identical tests
(TestJavaPortfolioPlaybookIncludesGovernanceAndContractRequirements,
TestJavaPortfolioPlaybookIncludesFallbackAndUnresolvedGuidance,
TestJavaPortfolioPlaybookCommandsAlignWithCurrentCLI) should be consolidated
into one table-driven test that uses t.Run("Should ...") subtests; replace the
three separate functions with a single TestJavaPortfolioPlaybookFragments that
calls readJavaPortfolioPlaybook once and iterates over a slice of cases where
each case has a name and requiredFragments slice, running each case as a subtest
(call t.Run(caseName, func(t *testing.T) { t.Parallel(); ... })) and performing
the strings.Contains checks inside the subtest; keep the helper
readJavaPortfolioPlaybook, preserve t.Parallel semantics at the subtest level,
and follow the t.Run naming convention and table-driven pattern as required.
In `@internal/cli/workflow_test_helpers_test.go`:
- Around line 324-508: Update the tests to use the required "Should..." subtest
naming pattern: in TestValidateJavaCodebaseSummary change the existing
t.Run(testCase.name, ...) to t.Run("Should "+testCase.name, ...) so every
table-driven case is named "Should ..."; also wrap the single-case
TestAssertJavaCodebaseSummaryPassesForValidInput body inside a t.Run("Should
pass for valid input", func(t *testing.T){ ... }) to follow the MUST use
t.Run("Should...") convention; keep existing use of t.Parallel() and
t.TempDir().
In `@internal/generate/benchmark_policy_test.go`:
- Around line 92-95: Replace the hardcoded "/tmp/canonical-repo" test input with
a t.TempDir()-based path: call tmp := t.TempDir(), build the repo path with
filepath.Join(tmp, "canonical-repo"), pass that into benchmarkGenerateOptions
and assert options.RootPath equals that joined path; also ensure the test
imports "path/filepath" if not already present and remove the hardcoded "/tmp"
usage.
In `@internal/generate/generate_integration_test.go`:
- Around line 179-260: The two new standalone tests
TestGenerateIntegrationDryRunSelectsJavaAdapterForMixedWorkspace and
TestGenerateIntegrationBuildsVaultFromJavaPhase2Workspace should be converted
into t.Run("Should...") subtests (and folded into a table-driven pattern where
appropriate) per test guidelines: wrap each scenario body in t.Run with
descriptive "Should..." names, move repeated setup into focused helpers (reuse
writeFixtureFile/writeJavaPhase2Fixture and newRunner), keep using t.TempDir for
isolation, and update any similar cases in the same file (around the 302-391
additions) to follow the same t.Run table-driven structure so all cases are
subtests rather than standalone Test* functions.
In `@internal/generate/generate_test.go`:
- Around line 213-283: The tests
TestSelectAdaptersIncludesJavaWhenWorkspaceHasJava,
TestSelectAdaptersForMixedWorkspace,
TestNewRunnerRegistersJavaAdapterInExpectedOrder, and
TestRunnerWithDefaultsIncludesJavaAdapterWhenAdaptersUnset should be converted
into table-driven t.Run suites to remove duplication: create a single test table
(cases with name, input languages, adapters, expected names/count/order) and
iterate with t.Run for the selectAdapters cases and another t.Run table for
runner/withDefaults adapter-order assertions; extract shared setup into small
helpers (e.g., makeFakeAdapters or adapterNames helper is fine) and use
t.TempDir() where filesystem isolation is needed, replacing repeated setup code,
ensuring each case uses t.Parallel() inside its t.Run body if appropriate.
In `@internal/generate/generate.go`:
- Around line 69-77: Extract the duplicated default adapters slice into a single
helper function (e.g., defaultLanguageAdapters) that returns
[]models.LanguageAdapter and replace the inline literal in newRunner (which sets
runner.adapters) and the other constructor that currently duplicates the slice
(the one around lines ~373-379) to call this helper; update the runner struct
initialization to use defaultLanguageAdapters() so all constructors share the
same source of truth and prevent drift when adding/removing adapters.
In `@internal/lint/lint_test.go`:
- Around line 197-325: Combine the four separate tests
(TestLintJavaDiagnosticsGovernanceParseErrorsBlockByDefault,
TestLintJavaDiagnosticsGovernanceFallbackDisabledByDefault,
TestLintJavaDiagnosticsGovernanceAppliesCustomFallbackThreshold,
TestLintJavaDiagnosticsGovernanceMessageUsesMachineReadableCounts) into a single
table-driven test that uses t.Run("Should ...") subtests and t.TempDir(),
extracting common setup (newTestTopic/writeMarkdownFile/mustLint or
LintWithOptions) into a helper; create a slice of cases with fields for input
metadata, optional LintOptions, expected presence/absence of models.LintIssue
(and expected Target/Severity/payload assertions for the machine-readable
message case), iterate cases and for each call t.Run(case.name, func(t
*testing.T){ ... }) performing the shared setup/assertions and only per-case
checks in the loop. Ensure you preserve the unique expectations for
JAVA_PARSE_ERROR, JAVA_RESOLUTION_FALLBACK and the JSON message assertion
(unmarshal parseIssue.Message) within the appropriate case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4ecfe5a8-e19d-41dd-9fa9-01aa05ec2146
⛔ Files ignored due to path filters (55)
.agents/skills/compozy/SKILL.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/cli-reference.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/config-reference.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/skills-reference.mdis excluded by!**/*.md,!.agents/**.agents/skills/compozy/references/workflow-guide.mdis excluded by!**/*.md,!.agents/**.compozy/tasks/java-ingest-adapter/_automation-json-contract.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/_java-portfolio-adoption-playbook.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/_meta.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/_phase3-benchmark-baseline.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/_prd.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/_rollout-mvp-signoff.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/_tasks.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/_techspec.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/adrs/adr-001.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/adrs/adr-002.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/adrs/adr-003.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/adrs/adr-004.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/adrs/adr-005.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/adrs/adr-006.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/MEMORY.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_01.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_02.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_03.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_04.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_05.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_06.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_07.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_08.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_09.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_10.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_11.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_12.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_13.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_14.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_15.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_16.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/memory/task_17.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_01.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_02.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_03.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_04.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_05.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_06.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_07.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_08.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_09.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_10.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_11.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_12.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_13.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_14.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_15.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_16.mdis excluded by!**/*.md.compozy/tasks/java-ingest-adapter/task_17.mdis excluded by!**/*.mdinternal/generate/testdata/java-benchmark-corpus/README.mdis excluded by!**/*.md
📒 Files selected for processing (31)
go.modinternal/adapter/go_adapter_test.gointernal/adapter/java_adapter.gointernal/adapter/java_adapter_integration_test.gointernal/adapter/java_adapter_test.gointernal/adapter/rust_adapter_test.gointernal/adapter/treesitter.gointernal/adapter/treesitter_test.gointernal/adapter/ts_adapter_test.gointernal/cli/generate_test.gointernal/cli/ingest_test.gointernal/cli/java_portfolio_playbook_integration_test.gointernal/cli/java_portfolio_playbook_test.gointernal/cli/lint.gointernal/cli/lint_test.gointernal/cli/workflow_integration_test.gointernal/cli/workflow_test_helpers_test.gointernal/generate/benchmark_policy.gointernal/generate/benchmark_policy_test.gointernal/generate/generate.gointernal/generate/generate_integration_test.gointernal/generate/generate_test.gointernal/lint/lint.gointernal/lint/lint_test.gointernal/models/kb_models.gointernal/models/kb_models_test.gointernal/models/models.gointernal/models/models_test.gointernal/scanner/scanner.gointernal/scanner/scanner_test.gointernal/vault/render.go
- Implement logic to ignore parent artifact ID in Maven POM parsing. - Add unit test for parsing Maven module POM signals. - Update existing tests for clarity and consistency in naming conventions. - Improve performance budget enforcement in integration tests for Java ingestion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Chores