atiaomar1978-hub commented on PR #24844: URL: https://github.com/apache/camel/pull/24844#issuecomment-5226880598
## Grok — test coverage review **Verdict: Good foundation, gaps on edge cases and half the producer surface at unit level** --- ### What is well covered - **`ApicurioRegistryProducerIT.testFullArtifactLifecycle`** — best test in the PR; exercises createGroup → create → get meta/content → update → listVersions → delete against real Apicurio 3.3.0. - **Mocked producer tests** — verify SDK call graph for 8/10 operations + header-driven operation selection. - **Mocked consumer tests** — latch-based `MockEndpoint.assertIsSatisfied` with timeout (good Camel convention). - **Test-infra module** — reusable `ApicurioRegistryService` + health-checked container; ITs are not one-offs. - **ComponentTest** — URI parsing, config copy, service location (often skipped in new components — nice to see). --- ### Gaps by risk | Risk | Gap | Suggested test | |------|-----|----------------| | **High** | No unit test for `validate` / `testCompatibility` | Mock dry-run POST success/failure; assert headers/body/`ApicurioRegistryValidationException` | | **High** | Validate IT never asserts `HEADER_VALIDATION_RESULT == true` or failure paths | Incompatible JSON schema + `failOnValidation=true` | | **Medium** | `searchArtifacts` route defined, never invoked in unit tests | One mock test with name/groupId query params | | **Medium** | Consumer watermark edge cases untested | Mock versions `[globalId=2, globalId=1]` — expect both or document sort requirement | | **Medium** | `fetchContent` consumer option | Unit + IT: body is stream/content vs `SearchedVersion` metadata | | **Low** | OIDC auth path | WireMock token endpoint or skip with `@DisabledIf` + documented manual QA | | **Low** | Header override resolution (`resolveGroupId` from header vs URI) | Single producer test overriding URI path via header | --- ### Test quality notes - ITs use JUnit assertions throughout; project prefers **AssertJ** in new tests (non-blocking). - `ApicurioRegistryConsumerIT` uses a private JUnit `assertEquals` helper — migrate to AssertJ when touching file. - Assertions are often **existence-only** (`assertNotNull`) where **value** assertions would catch regressions (validation result, compatibility boolean). --- ### Scorecard | Layer | Score | Notes | |-------|-------|-------| | Unit breadth | **7/10** | 8/10 ops + 2 consumer scenarios | | Unit depth | **5/10** | Few header/error-path assertions | | Integration | **8/10** | Strong lifecycle; weak validate/consumer multi-version | | Infra | **9/10** | Proper test-infra split | | **Overall** | **7/10** | Acceptable for Preview; tighten before stable | _AI-generated Grok test-coverage review on behalf of atiaomar1978-hub._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
