Andrea Cosentino created CAMEL-24225:
----------------------------------------
Summary: 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
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)
