diff --git a/docs/coverage-map-follow-ups.md b/docs/coverage-map-follow-ups.md new file mode 100644 index 00000000..d3f73624 --- /dev/null +++ b/docs/coverage-map-follow-ups.md @@ -0,0 +1,60 @@ +# Coverage-map follow-up backlog + +This document records implementation requirements established after coverage-map schema v1. Detailed +experimental evidence remains with the POC that produced it; this file contains only the consequences +for the corresponding backlog items. + +## Task 5c — Coverage runtime and fragment production + +Status: **NOT STARTED — blocking collector finding recorded** + +The three-agent Spring PetClinic POC exposed an unsafe behavior in the current experimental collector. +Run-wide setup attribution can create false setup-to-test relations because it combines every setup +class observed in a test JVM with every test container observed in that JVM. + +The PetClinic baseline produced three false setup edges that were absent from the distributed result: + +- `CacheConfiguration -> ValidatorTests` +- `CrashController -> ValidatorTests` +- `WelcomeController -> ValidatorTests` + +Task 5c must attribute setup coverage to the actual JUnit lifecycle/container scope. It must not derive +affected tests from a run-wide Cartesian product. The existing schema-v1 `SetupScope` model appears able +to represent the required narrower attribution; the POC did not establish a need for another schema. + +Additional acceptance criterion: + +```text +single-shard and multi-shard collection must produce +semantically identical setup coverage +``` + +## Task 5d — Fragment merge, publication, and Jenkins orchestration + +Status: **NOT STARTED — POC proven** + +The three-agent Spring PetClinic POC proved the mechanism for: + +- independent expected inventory before collection +- deterministic sharding +- parallel execution across three isolated Jenkins agents +- sequential tests inside each collector context +- workspace-relative fragment production +- fragment transport using Jenkins stash/unstash +- fragment join on an explicit agent +- completeness validation against expected tests and shards +- exact revision consistency +- detection of missing, duplicate, and unexpected join input + +This evidence does not complete Task 5d. Production storage, retention, lookup, scheduling, publication +policy, failure recovery, and CI-wide configuration remain unimplemented. + +## Canonical POC conclusion + +> Distributed mapping itself did not introduce coverage loss. The only semantic difference was caused +> by an existing run-wide setup attribution defect in the collector. + +The evidence is documented in `stp-jenkins-plugin-poc/docs/coverage-mapping-poc.md`: both complete maps +contained the same 58-test inventory, 54 mapped tests, four explicitly skipped tests, 338 test-to-class +edges, and 1,045 test-to-method edges. Setup comparison alone differed: 304 baseline edges versus 301 +distributed edges. diff --git a/docs/coverage-map-model.md b/docs/coverage-map-model.md index c5fe5017..022e2052 100644 --- a/docs/coverage-map-model.md +++ b/docs/coverage-map-model.md @@ -11,6 +11,13 @@ The main types are `CoverageMap`, `CoverageMapRevision`, `TestIdentity`, `TestCo `CollectionSummary`, `Completeness`, `TestOutcome`, `GeneratorProvenance`, `MapStatistics`, `CoverageFragment`, and `MethodCoverageReference`. +`TestIdentity` is the structured pair of a fully-qualified JVM binary class name and the exact +declared runtime method name. Method names are not restricted to Java source identifiers: legal +JUnit/JVM-language names such as Kotlin backtick names containing spaces are preserved verbatim. +The textual form separates the class from the method at the first `#`; `#` remains legal inside +the method suffix because it is not legal in the class-name portion. Empty names, ISO control +characters, and the JVM-unqualified-name forbidden characters `.`, `;`, `[`, and `/` are rejected. + ## Setup is first-class Static initialization, `@BeforeAll`, inherited setup, nested setup, and cached framework contexts do @@ -52,6 +59,9 @@ test discovery (5c), setup-scope or unmapped-test selection policy (5b), storage Jenkins orchestration (5d), or arbitrary base/head revision handling (5e). It also does not alter Gradle or Maven adapters. +Implementation findings and POC-proven boundaries for 5c and 5d are tracked in +[`coverage-map-follow-ups.md`](coverage-map-follow-ups.md). + ## Version impact Schema v1 is not compatible with legacy maps. Legacy test keys use `SimpleClass#method_7hexHash`, diff --git a/smart-test-picker-common/src/main/java/com/sap/oss/smarttestpicker/coverage/model/TestIdentity.java b/smart-test-picker-common/src/main/java/com/sap/oss/smarttestpicker/coverage/model/TestIdentity.java index 3cabcce8..ee51f255 100644 --- a/smart-test-picker-common/src/main/java/com/sap/oss/smarttestpicker/coverage/model/TestIdentity.java +++ b/smart-test-picker-common/src/main/java/com/sap/oss/smarttestpicker/coverage/model/TestIdentity.java @@ -9,13 +9,12 @@ public record TestIdentity(String className, String methodName) implements Comparable { private static final Pattern CLASS_NAME = Pattern.compile("[\\p{javaJavaIdentifierStart}][\\p{javaJavaIdentifierPart}]*(?:[.$][\\p{javaJavaIdentifierStart}][\\p{javaJavaIdentifierPart}]*)+"); - private static final Pattern METHOD_NAME = Pattern.compile("[\\p{javaJavaIdentifierStart}][\\p{javaJavaIdentifierPart}]*"); public TestIdentity { if (className == null || !CLASS_NAME.matcher(className).matches()) throw new IllegalArgumentException("Malformed test class name: " + className); - if (methodName == null || !METHOD_NAME.matcher(methodName).matches()) + if (methodName == null || methodName.isEmpty() || methodName.codePoints().anyMatch(TestIdentity::invalidMethodCharacter)) throw new IllegalArgumentException("Malformed test method name: " + methodName); } @@ -23,11 +22,20 @@ public static TestIdentity parse(String value) { Objects.requireNonNull(value, "value"); int separator = value.indexOf('#'); - if (separator <= 0 || separator != value.lastIndexOf('#') || separator == value.length() - 1) + if (separator <= 0 || separator == value.length() - 1) throw new IllegalArgumentException("Malformed test identity: " + value); return new TestIdentity(value.substring(0, separator), value.substring(separator + 1)); } + private static boolean invalidMethodCharacter(int character) + { + return Character.isISOControl(character) || character == '.' || character == ';' || character == '[' || character == '/'; + } + @Override public String toString() { return className + "#" + methodName; } - @Override public int compareTo(TestIdentity other) { return toString().compareTo(other.toString()); } + @Override public int compareTo(TestIdentity other) + { + int byClass = className.compareTo(other.className); + return byClass != 0 ? byClass : methodName.compareTo(other.methodName); + } } diff --git a/smart-test-picker-common/src/test/java/com/sap/oss/smarttestpicker/coverage/CoverageMapContractTest.java b/smart-test-picker-common/src/test/java/com/sap/oss/smarttestpicker/coverage/CoverageMapContractTest.java index 7fa8d61e..bf14e239 100644 --- a/smart-test-picker-common/src/test/java/com/sap/oss/smarttestpicker/coverage/CoverageMapContractTest.java +++ b/smart-test-picker-common/src/test/java/com/sap/oss/smarttestpicker/coverage/CoverageMapContractTest.java @@ -62,6 +62,52 @@ class CoverageMapContractTest assertThrows(IllegalArgumentException.class, () -> TestIdentity.parse("FooTest#test")); } + @Test void testIdentityLosslesslySupportsJvmAndKotlinMethodNames() + { + List names = List.of( + "testSomething", + "Register reflection hints for Kotlin data class", + "multiple spaces", + "punctuation!?+-=():,'\"@%^&*|~`{}<>", + "foo#bar"); + for (String name : names) + { + TestIdentity identity = new TestIdentity("com.foo.MyTests", name); + assertEquals(identity, TestIdentity.parse(identity.toString())); + assertEquals(identity.hashCode(), TestIdentity.parse(identity.toString()).hashCode()); + } + + TestIdentity springCore = TestIdentity.parse("org.springframework.aot.hint.BindingReflectionHintsRegistrarKotlinTests#Register reflection hints for Kotlin data class"); + assertEquals("Register reflection hints for Kotlin data class", springCore.methodName()); + assertEquals(2, Set.of(new TestIdentity("com.foo.MyTests", "foo bar"), + new TestIdentity("com.foo.MyTests", "foo_bar")).size()); + assertThrows(IllegalArgumentException.class, () -> new TestIdentity("com.foo.MyTests", "")); + assertThrows(IllegalArgumentException.class, () -> new TestIdentity("com.foo.MyTests", "line\nbreak")); + for (String forbidden : List.of("has.dot", "has;semicolon", "has[bracket", "has/slash")) + assertThrows(IllegalArgumentException.class, () -> new TestIdentity("com.foo.MyTests", forbidden)); + } + + @Test void extendedTestIdentitiesRoundTripThroughFragmentAndCoverageMap() + { + TestIdentity kotlin = new TestIdentity("com.foo.MyTests", "Register reflection hints for Kotlin data class"); + TestIdentity punctuation = new TestIdentity("com.foo.OuterTests$Nested", "foo#bar !?"); + Map tests = Map.of(kotlin, covered("com.foo.Service"), + punctuation, covered("com.foo.Other")); + + CoverageFragment fragment = new CoverageFragment(1, new CoverageMapRevision("abc123"), + new ShardId("0"), tests, List.of(), List.of(), true); + CoverageFragment decodedFragment = new CoverageFragmentCodec().deserialize( + new CoverageFragmentCodec().serialize(fragment)); + assertEquals(tests, decodedFragment.tests()); + + CoverageMap original = map(tests, List.of(), List.of()); + CoverageMapCodec codec = new CoverageMapCodec(); + byte[] serialized = codec.serialize(original); + CoverageMap decodedMap = codec.deserialize(serialized); + assertEquals(tests, decodedMap.tests()); + assertArrayEquals(serialized, codec.serialize(decodedMap)); + } + @Test void constructedMapIsImmutable() { Map mutable = new HashMap<>(); mutable.put(FOO, covered("com.foo.Service"));