[
https://issues.apache.org/jira/browse/CAMEL-24225?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18097911#comment-18097911
]
Andrea Cosentino commented on CAMEL-24225:
------------------------------------------
Fixed via https://github.com/apache/camel/pull/24983 (merged as cfab3c41f519).
> AGENTS.md test-visibility guidance mandates non-compiling code (@Override,
> broken example, no base-class carve-out)
> -------------------------------------------------------------------------------------------------------------------
>
> Key: CAMEL-24225
> URL: https://issues.apache.org/jira/browse/CAMEL-24225
> Project: Camel
> Issue Type: Bug
> Reporter: Andrea Cosentino
> Assignee: Andrea Cosentino
> Priority: Major
> Fix For: 4.22.0
>
>
> The "Test Visibility: Drop {{public}} From Test Classes and Methods" section
> added to {{AGENTS.md}} (symlinked as {{CLAUDE.md}}) contains guidance that
> produces code which does not compile. Because this file is consumed by AI
> coding agents as normative instructions, the defects propagate directly into
> contributions.
> h3. 1. The rule for {{@Override}} methods is illegal Java
> {{AGENTS.md}} line 279:
> {quote}
> {{@BeforeAll}}, {{@AfterAll}}, {{@BeforeEach}}, {{@AfterEach}}, and
> {{@Override}} methods follow the same rule: drop {{public}} when adding or
> modifying them.
> {quote}
> Reducing the visibility of an overriding method is forbidden by JLS 8.4.8.3.
> Verified with javac:
> {noformat}
> error: configure() in Sub cannot override configure() in Base
> attempting to assign weaker access privileges; was public
> {noformat}
> This applies to {{public void configure()}} in every anonymous
> {{RouteBuilder}} — {{RouteBuilder.configure()}} is declared {{public
> abstract}} ({{core/camel-core-model/.../RouteBuilder.java}} line 200), and
> there are roughly 8,900 such occurrences under {{src/test}}. It also applies
> to the ~27 {{src/test}} overrides of {{configureContext}} /
> {{configureTest}}, which implement interface methods and are therefore
> implicitly public.
> Separately, the {{@Before*}}/{{@After*}} half of the rule is close to vacuous
> for Camel tests: {{CamelTestSupport}} declares {{public final void setUp()}}
> and {{public final void tearDown(TestInfo)}} (not overridable), while
> {{doPreSetup}}/{{doPostSetup}} are already {{protected}}.
> h3. 2. The "Preferred" example does not compile
> {{AGENTS.md}} lines 256-263:
> {noformat}
> class MyComponentTest extends CamelTestSupport {
> @Test
> void testSendMessage() { ... }
> @Override
> void configure() throws Exception { ... }
> }
> {noformat}
> {{CamelTestSupport}} has no {{configure()}} method at all — only
> {{configureContext()}} (line 81) and {{configureTest()}} (line 97) — so the
> {{@Override}} fails with "method does not override or implement a method from
> a supertype". Read instead as {{RouteBuilder.configure()}}, it fails with the
> weaker-access error above.
> The correct idiom is already visible in {{CamelTestSupport.java}} line 356,
> where {{configure()}} stays {{public}} inside the anonymous {{RouteBuilder}}.
> h3. 3. No carve-out for cross-package or published test base classes
> Line 274 states unconditionally:
> {quote}
> New test classes and test methods MUST NOT use the {{public}} modifier.
> {quote}
> A package-private class cannot be extended from another package. In this repo
> there are 183 {{public abstract class}} declarations under {{src/test}} and
> ~70 distinct {{*TestSupport}} / {{*ITSupport}} / {{*TestBase}} types imported
> cross-package. {{ContextTestSupport}} alone is imported ~2,500 times,
> including from other modules via the published test-jar (8 modules attach a
> test-jar; 35 consume one).
> More seriously, "test classes" is ambiguous enough that an agent could apply
> the rule to {{components/camel-test/camel-test-junit5|junit6}} and
> {{test-infra/**}}. Those live in {{src/main}}, deploy normally, and are
> consumed by camel-quarkus, camel-spring-boot and by users' own tests —
> stripping {{public}} there would be a breaking API change.
> h3. Suggested fix
> Three edits, keeping the section:
> # Replace the broken example with the real idiom ({{createRouteBuilder()}}
> returning an anonymous {{RouteBuilder}} whose {{configure()}} stays
> {{public}}).
> # Remove {{@Override}} from the line-279 list, or qualify it: "unless the
> method overrides or implements a public supertype method, where Java forbids
> reducing visibility".
> # Add an exception to line 274 for abstract/base test classes extended from
> another package or module, and for anything under
> {{components/camel-test/**}} or {{test-infra/**}}.
> The underlying convention is sound — JUnit 5 does not require {{public}}, and
> the repo has 751 package-private test classes already. Only the three
> statements above need correcting.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
