|
|
|
@@ -0,0 +1,144 @@
|
|
|
|
|
package dev.ltms.fleet.peer;
|
|
|
|
|
|
|
|
|
|
import org.junit.jupiter.api.Test;
|
|
|
|
|
|
|
|
|
|
import java.lang.reflect.Constructor;
|
|
|
|
|
import java.lang.reflect.RecordComponent;
|
|
|
|
|
import java.util.ArrayList;
|
|
|
|
|
import java.util.Arrays;
|
|
|
|
|
import java.util.LinkedHashMap;
|
|
|
|
|
import java.util.List;
|
|
|
|
|
import java.util.Locale;
|
|
|
|
|
import java.util.Map;
|
|
|
|
|
import java.util.Objects;
|
|
|
|
|
import java.util.Set;
|
|
|
|
|
import java.util.TreeSet;
|
|
|
|
|
|
|
|
|
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Fleetd #382, the same "defect factory" #357 and #358 guarded on {@code FleetConfig.withDefaults()}
|
|
|
|
|
* and {@link dev.ltms.fleet.session.MemberSession}'s rebuild sites — reproduced here on
|
|
|
|
|
* {@link SpawnRequest#withProfile(String)}.
|
|
|
|
|
*
|
|
|
|
|
* <p>{@code SpawnRequest} has back-compat constructors at arity 3 and 5 alongside its canonical
|
|
|
|
|
* arity-6 constructor. Before this ticket, {@code CompositePeerLauncher} routed a profile by
|
|
|
|
|
* building a fresh {@code SpawnRequest} from a literal {@code new SpawnRequest(...)} call listing
|
|
|
|
|
* six of the original request's own accessors. That call is only correct because it happens to
|
|
|
|
|
* name exactly six arguments today — add a 7th component and the established back-compat pattern
|
|
|
|
|
* (a new constructor at the old, now-shorter arity) and a call one argument short of the new
|
|
|
|
|
* canonical arity would silently rebind to that back-compat constructor, dropping the new
|
|
|
|
|
* component on every profile-routed spawn without any compile error. {@link #withProfile} replaces
|
|
|
|
|
* that literal call, so this test guards the ONE rebuild site instead of a call site scattered
|
|
|
|
|
* through a launcher.
|
|
|
|
|
*
|
|
|
|
|
* <p>The check below builds a {@link SpawnRequest} through the TRUE canonical constructor —
|
|
|
|
|
* resolved by the record's own component types via {@code getDeclaredConstructor}, never by
|
|
|
|
|
* argument count, so it can never itself land on a back-compat overload — with a real, distinctive,
|
|
|
|
|
* non-null value in every component, calls {@link SpawnRequest#withProfile(String)}, and asserts
|
|
|
|
|
* every component the method is not documented to change survives unchanged, while {@code
|
|
|
|
|
* profileName} comes back as the new value it was given. A component that comes back anything else
|
|
|
|
|
* was silently dropped — the shape of the defect this test exists to catch.
|
|
|
|
|
*
|
|
|
|
|
* <p>{@link #EXCLUDED_FROM_SURVIVAL_CHECK} is kept deliberately empty and size-pinned by
|
|
|
|
|
* {@link #exclusionListSizeIsPinned()}, for the same reason the other two guards pin theirs at
|
|
|
|
|
* zero: a checker whose escape hatch can grow to silence a failure is not a checker. Every one of
|
|
|
|
|
* {@link SpawnRequest}'s 6 current components has a real, non-null value here and none is excluded.
|
|
|
|
|
*/
|
|
|
|
|
class SpawnRequestWithProfilePreservesEveryComponentTest {
|
|
|
|
|
|
|
|
|
|
private static final RecordComponent[] COMPONENTS = SpawnRequest.class.getRecordComponents();
|
|
|
|
|
|
|
|
|
|
/** Deliberately empty today; grow it only with a matching justification, and re-pin the size. */
|
|
|
|
|
private static final Set<String> EXCLUDED_FROM_SURVIVAL_CHECK = Set.of();
|
|
|
|
|
|
|
|
|
|
/** One real, distinctive, non-null value per component — none of the 6 is excluded. */
|
|
|
|
|
private static Map<String, Object> baseValues() {
|
|
|
|
|
Map<String, Object> v = new LinkedHashMap<>();
|
|
|
|
|
v.put("profileName", "profile-guard");
|
|
|
|
|
v.put("requestedCwd", "/wt/requested-guard");
|
|
|
|
|
v.put("callerCwd", "/wt/caller-guard");
|
|
|
|
|
v.put("sessionName", "session-guard");
|
|
|
|
|
v.put("resumeSessionId", "resume-guard");
|
|
|
|
|
v.put("role", MemberRole.REVIEWER);
|
|
|
|
|
assertNamesMatchComponents(v);
|
|
|
|
|
return v;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Guards {@link #baseValues()} itself against drifting from the record's real shape — forgetting
|
|
|
|
|
* to add a new component here fails this assertion by name, rather than silently checking one
|
|
|
|
|
* component fewer than the record has.
|
|
|
|
|
*/
|
|
|
|
|
private static void assertNamesMatchComponents(Map<String, Object> values) {
|
|
|
|
|
Set<String> names = new TreeSet<>();
|
|
|
|
|
for (RecordComponent rc : COMPONENTS) {
|
|
|
|
|
names.add(rc.getName());
|
|
|
|
|
}
|
|
|
|
|
assertEquals(names, new TreeSet<>(values.keySet()),
|
|
|
|
|
"this test's value map has drifted from SpawnRequest's actual components — "
|
|
|
|
|
+ "update baseValues() alongside the record");
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/**
|
|
|
|
|
* Builds a {@link SpawnRequest} through the TRUE canonical constructor — resolved by the
|
|
|
|
|
* record's own component types, not by argument count — so this never accidentally exercises a
|
|
|
|
|
* back-compat overload the way a literal {@code new SpawnRequest(...)} call risks doing.
|
|
|
|
|
*/
|
|
|
|
|
private static SpawnRequest requestOf(Map<String, Object> values) throws ReflectiveOperationException {
|
|
|
|
|
Class<?>[] types = Arrays.stream(COMPONENTS).map(RecordComponent::getType).toArray(Class<?>[]::new);
|
|
|
|
|
Object[] args = Arrays.stream(COMPONENTS).map(rc -> values.get(rc.getName())).toArray();
|
|
|
|
|
Constructor<SpawnRequest> ctor = SpawnRequest.class.getDeclaredConstructor(types);
|
|
|
|
|
return ctor.newInstance(args);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
@Test
|
|
|
|
|
void exclusionListSizeIsPinned() {
|
|
|
|
|
assertEquals(0, EXCLUDED_FROM_SURVIVAL_CHECK.size(),
|
|
|
|
|
"EXCLUDED_FROM_SURVIVAL_CHECK grew from 0 — every entry needs a justification in "
|
|
|
|
|
+ "this test class's javadoc AND this assertion re-pinned to the new size; a "
|
|
|
|
|
+ "growing exclusion list that silences failures on its own is not a guard");
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
@Test
|
|
|
|
|
void withProfilePreservesEveryOtherComponent() throws ReflectiveOperationException {
|
|
|
|
|
Map<String, Object> base = baseValues();
|
|
|
|
|
SpawnRequest request = requestOf(base);
|
|
|
|
|
SpawnRequest result = request.withProfile("profile-updated");
|
|
|
|
|
|
|
|
|
|
Map<String, Object> expectedOverrides = Map.of("profileName", "profile-updated");
|
|
|
|
|
|
|
|
|
|
List<String> dropped = new ArrayList<>();
|
|
|
|
|
int checked = 0;
|
|
|
|
|
for (RecordComponent rc : COMPONENTS) {
|
|
|
|
|
String name = rc.getName();
|
|
|
|
|
if (EXCLUDED_FROM_SURVIVAL_CHECK.contains(name)) {
|
|
|
|
|
continue;
|
|
|
|
|
}
|
|
|
|
|
checked++;
|
|
|
|
|
Object expected = expectedOverrides.containsKey(name) ? expectedOverrides.get(name) : base.get(name);
|
|
|
|
|
Object actual;
|
|
|
|
|
try {
|
|
|
|
|
actual = rc.getAccessor().invoke(result);
|
|
|
|
|
} catch (ReflectiveOperationException e) {
|
|
|
|
|
throw new RuntimeException("failed to read SpawnRequest." + name + "()", e);
|
|
|
|
|
}
|
|
|
|
|
if (!Objects.equals(expected, actual)) {
|
|
|
|
|
dropped.add(String.format(Locale.ROOT,
|
|
|
|
|
"%s: withProfile() was expected to carry (%s) for '%s' but returned %s — a "
|
|
|
|
|
+ "component silently dropped by withProfile(), the shape of the defect "
|
|
|
|
|
+ "this test exists to catch (its final \"return new SpawnRequest(...)\" "
|
|
|
|
|
+ "call binding to a back-compat constructor instead of the true "
|
|
|
|
|
+ "canonical one)",
|
|
|
|
|
name, expected, name, actual));
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
System.out.printf(Locale.ROOT,
|
|
|
|
|
"SpawnRequest.withProfile() component-survival coverage — %d components, %d checked, "
|
|
|
|
|
+ "%d excluded, %d survived%n",
|
|
|
|
|
COMPONENTS.length, checked, EXCLUDED_FROM_SURVIVAL_CHECK.size(), checked - dropped.size());
|
|
|
|
|
assertEquals(List.of(), dropped,
|
|
|
|
|
"withProfile() silently dropped these components: " + dropped);
|
|
|
|
|
}
|
|
|
|
|
}
|
Dai Ha