Remove TestIdentifier.SerializedForm (#4913)

This commit removes the custom serialization support for 
`TestIdentifier` which was added to preserve backwards compatibility 
which is no longer necessary for 6.0.

Resolves #4882.

---------

Co-authored-by: Marc Philipp <mail@marcphilipp.de>

Author: mehulimukherjee

documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC3.adoc

@@ -21,7 +21,8 @@ JUnit repository on GitHub.
 [[release-notes-6.0.0-RC3-junit-platform-deprecations-and-breaking-changes]]
 ==== Deprecations and Breaking Changes
 
-* ❓
+* Change serialization of `TestIdentifier` in a backwards-incompatible way to simplify
+  its implementation
 
 [[release-notes-6.0.0-RC3-junit-platform-new-features-and-improvements]]
 ==== New Features and Improvements

junit-platform-launcher/src/main/java/org/junit/platform/launcher/TestIdentifier.java

@@ -16,11 +16,6 @@
 import static org.apiguardian.api.API.Status.STABLE;
 import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement;
 
-import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
-import java.io.ObjectStreamClass;
-import java.io.ObjectStreamField;
 import java.io.Serial;
 import java.io.Serializable;
 import java.util.LinkedHashSet;
@@ -49,23 +44,21 @@
 public final class TestIdentifier implements Serializable {
 
 	@Serial
-	private static final long serialVersionUID = 1L;
-	@Serial
-	@SuppressWarnings("UnusedVariable")
-	private static final ObjectStreamField[] serialPersistentFields = ObjectStreamClass.lookup(
-		SerializedForm.class).getFields();
+	private static final long serialVersionUID = 2L;
+
+	private final UniqueId uniqueId;
+
+	private final @Nullable UniqueId parentId;
 
-	// These are effectively final but not technically due to late initialization when deserializing
-	private /* final */ UniqueId uniqueId;
+	private final String displayName;
+	private final String legacyReportingName;
 
-	private /* final */ @Nullable UniqueId parentId;
+	private final @Nullable TestSource source;
 
-	private /* final */ String displayName;
-	private /* final */ String legacyReportingName;
+	@SuppressWarnings("serial") // Declared type is Set (not Serializable); actual instances are Serializable.
+	private final Set<TestTag> tags;
 
-	private /* final */ @Nullable TestSource source;
-	private /* final */ Set<TestTag> tags;
-	private /* final */ Type type;
+	private final Type type;
 
 	/**
 	 * Factory for creating a new {@link TestIdentifier} from a {@link TestDescriptor}.
@@ -272,87 +265,4 @@ public String toString() {
 		// @formatter:on
 	}
 
-	@Serial
-	private void writeObject(ObjectOutputStream s) throws IOException {
-		SerializedForm serializedForm = new SerializedForm(this);
-		serializedForm.serialize(s);
-	}
-
-	@Serial
-	private void readObject(ObjectInputStream s) throws ClassNotFoundException, IOException {
-		SerializedForm serializedForm = SerializedForm.deserialize(s);
-		uniqueId = UniqueId.parse(serializedForm.uniqueId);
-		displayName = serializedForm.displayName;
-		source = serializedForm.source;
-		tags = serializedForm.tags;
-		type = serializedForm.type;
-		String parentId = serializedForm.parentId;
-		this.parentId = parentId == null ? null : UniqueId.parse(parentId);
-		legacyReportingName = serializedForm.legacyReportingName;
-	}
-
-	/**
-	 * Represents the serialized output of {@code TestIdentifier}. The fields on this
-	 * class match the fields that {@code TestIdentifier} had prior to 1.8.
-	 */
-	private static class SerializedForm implements Serializable {
-
-		@Serial
-		private static final long serialVersionUID = 1L;
-
-		private final String uniqueId;
-
-		@Nullable
-		private final String parentId;
-
-		private final String displayName;
-		private final String legacyReportingName;
-
-		@Nullable
-		private final TestSource source;
-
-		@SuppressWarnings({ "serial", "RedundantSuppression" }) // always used with serializable implementation (see TestIdentifier#copyOf())
-		private final Set<TestTag> tags;
-		private final Type type;
-
-		SerializedForm(TestIdentifier testIdentifier) {
-			this.uniqueId = testIdentifier.uniqueId.toString();
-			UniqueId parentId = testIdentifier.parentId;
-			this.parentId = parentId == null ? null : parentId.toString();
-			this.displayName = testIdentifier.displayName;
-			this.legacyReportingName = testIdentifier.legacyReportingName;
-			this.source = testIdentifier.source;
-			this.tags = testIdentifier.tags;
-			this.type = testIdentifier.type;
-		}
-
-		@SuppressWarnings("unchecked")
-		private SerializedForm(ObjectInputStream.GetField fields) throws IOException, ClassNotFoundException {
-			this.uniqueId = (String) fields.get("uniqueId", null);
-			this.parentId = (String) fields.get("parentId", null);
-			this.displayName = (String) fields.get("displayName", null);
-			this.legacyReportingName = (String) fields.get("legacyReportingName", null);
-			this.source = (TestSource) fields.get("source", null);
-			this.tags = (Set<TestTag>) fields.get("tags", null);
-			this.type = (Type) fields.get("type", null);
-		}
-
-		void serialize(ObjectOutputStream s) throws IOException {
-			ObjectOutputStream.PutField fields = s.putFields();
-			fields.put("uniqueId", uniqueId);
-			fields.put("parentId", parentId);
-			fields.put("displayName", displayName);
-			fields.put("legacyReportingName", legacyReportingName);
-			fields.put("source", source);
-			fields.put("tags", tags);
-			fields.put("type", type);
-			s.writeFields();
-		}
-
-		static SerializedForm deserialize(ObjectInputStream s) throws IOException, ClassNotFoundException {
-			ObjectInputStream.GetField fields = s.readFields();
-			return new SerializedForm(fields);
-		}
-	}
-
 }

platform-tests/src/test/java/org/junit/platform/launcher/TestIdentifierTests.java

@@ -10,15 +10,25 @@
 
 package org.junit.platform.launcher;
 
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.toSet;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.platform.commons.util.SerializationUtils.deserialize;
 import static org.junit.platform.commons.util.SerializationUtils.serialize;
 
+import java.io.Serializable;
+import java.util.AbstractSet;
+import java.util.Iterator;
 import java.util.Set;
+import java.util.stream.IntStream;
 
+import org.jspecify.annotations.NullMarked;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestTag;
 import org.junit.platform.engine.UniqueId;
@@ -30,6 +40,7 @@
 /**
  * @since 1.0
  */
+@NullMarked
 class TestIdentifierTests {
 
 	@Test
@@ -56,20 +67,37 @@ void inheritsTypeFromDescriptor() {
 		assertTrue(identifier.isContainer());
 	}
 
-	@Test
-	void currentVersionCanBeSerializedAndDeserialized() throws Exception {
-		var originalIdentifier = createOriginalTestIdentifier();
-		var deserializedIdentifier = (TestIdentifier) deserialize(serialize(originalIdentifier));
-		assertDeepEquals(originalIdentifier, deserializedIdentifier);
+	@ParameterizedTest
+	@ValueSource(ints = { 0, 1, 2 })
+	void currentVersionCanBeSerializedAndDeserialized(int tagCount) throws Exception {
+		var tags = IntStream.range(0, tagCount) //
+				.mapToObj(i -> TestTag.create("tag-" + i)) //
+				.collect(collectingAndThen(toSet(), TestIdentifierTests::unserializableSet));
+
+		var original = createOriginalTestIdentifier(tags);
+
+		byte[] bytes = serialize(original);
+		var roundTripped = (TestIdentifier) deserialize(bytes);
+
+		assertDeepEquals(original, roundTripped);
+		assertThat(original.getTags()).isInstanceOf(Serializable.class);
 	}
 
-	@Test
-	void initialVersionCanBeDeserialized() throws Exception {
-		try (var inputStream = getClass().getResourceAsStream("/serialized-test-identifier")) {
-			var bytes = inputStream.readAllBytes();
-			var deserializedIdentifier = (TestIdentifier) deserialize(bytes);
-			assertDeepEquals(createOriginalTestIdentifier(), deserializedIdentifier);
-		}
+	private static <T> Set<T> unserializableSet(Set<T> delegate) {
+		var wrapper = new AbstractSet<T>() {
+
+			@Override
+			public Iterator<T> iterator() {
+				return delegate.iterator();
+			}
+
+			@Override
+			public int size() {
+				return delegate.size();
+			}
+		};
+		assertThat(wrapper).isNotInstanceOf(Serializable.class);
+		return wrapper;
 	}
 
 	@Test
@@ -100,12 +128,12 @@ private static void assertDeepEquals(TestIdentifier first, TestIdentifier second
 		assertEquals(first.getParentIdObject(), second.getParentIdObject());
 	}
 
-	private static TestIdentifier createOriginalTestIdentifier() {
+	private static TestIdentifier createOriginalTestIdentifier(Set<TestTag> tags) {
 		var engineDescriptor = new EngineDescriptor(UniqueId.forEngine("engine"), "Engine");
 		var uniqueId = engineDescriptor.getUniqueId().append("child", "child");
 		var testSource = ClassSource.from(TestIdentifierTests.class);
-		var testDescriptor = new AbstractTestDescriptor(uniqueId, "displayName", testSource) {
 
+		var testDescriptor = new AbstractTestDescriptor(uniqueId, "displayName", testSource) {
 			@Override
 			public Type getType() {
 				return Type.TEST;
@@ -118,9 +146,10 @@ public String getLegacyReportingName() {
 
 			@Override
 			public Set<TestTag> getTags() {
-				return Set.of(TestTag.create("aTag"));
+				return tags;
 			}
 		};
+
 		engineDescriptor.addChild(testDescriptor);
 		return TestIdentifier.from(testDescriptor);
 	}