Merge pull request #838 from retest/feature/update-kryo

Feature/update kryo

CHANGELOG.md

@@ -26,6 +26,7 @@ Table of Contents
 ### Breaking Changes
 
 * Since 01.12.2020 this project is owned and developed by [UBS Hainer GmbH](https://ubs-hainer.com/).
+* Update the storage library used for reports (kryo 4 -> 5), it's possible that older test reports can't longer be read.
 
 ### Bug Fixes
 

pom.xml

@@ -98,16 +98,15 @@
 		</dependency>
 
 		<dependency>
-			<groupId>com.esotericsoftware</groupId>
-			<artifactId>kryo</artifactId>
-			<version>4.0.2</version>
+			<groupId>com.esotericsoftware.kryo</groupId>
+			<artifactId>kryo5</artifactId>
+			<version>5.0.3</version>
 		</dependency>
 
 		<dependency>
 			<groupId>de.javakaffee</groupId>
 			<artifactId>kryo-serializers</artifactId>
-			<!-- last version compatible with Kryo v4.0.2 -->
-			<version>0.43</version>
+			<version>0.45</version>
 		</dependency>
 
 		<dependency>

src/main/java/de/retest/recheck/persistence/bin/KryoPersistence.java

@@ -11,6 +11,7 @@
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.regex.Matcher;
@@ -29,7 +30,9 @@
 import de.retest.recheck.persistence.IncompatibleReportVersionException;
 import de.retest.recheck.persistence.Persistable;
 import de.retest.recheck.persistence.Persistence;
+import de.retest.recheck.report.SuiteReplayResult;
 import de.retest.recheck.report.TestReport;
+import de.retest.recheck.ui.review.GoldenMasterSource;
 import de.retest.recheck.util.FileUtil;
 import de.retest.recheck.util.VersionProvider;
 import lombok.extern.slf4j.Slf4j;
@@ -74,10 +77,17 @@ public KryoPersistence() {
 
 	private static Kryo createKryo() {
 		final Kryo kryo = new Kryo();
+		kryo.setInstantiatorStrategy( new StdInstantiatorStrategy() );
 
-		kryo.setInstantiatorStrategy( new Kryo.DefaultInstantiatorStrategy( new StdInstantiatorStrategy() ) );
+		kryo.setRegistrationRequired( false ); // TODO remove, see #836
 
-		final Registration registration = kryo.getRegistration( TreeMultiset.class );
+		kryo.register( TestReport.class );
+		kryo.register( GoldenMasterSource.class );
+		kryo.register( SuiteReplayResult.class );
+		kryo.register( ArrayList.class );
+		// TODO add more, see #836
+
+		final Registration registration = kryo.register( TreeMultiset.class );
 		registration.setInstantiator( TreeMultiset::create );
 
 		UnmodifiableCollectionsSerializer.registerSerializers( kryo );

src/test/java/de/retest/recheck/RecheckImplIT.java

@@ -1,8 +1,6 @@
 package de.retest.recheck;
 
 import static org.junit.jupiter.api.Assertions.fail;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.spy;
 
 import java.util.Arrays;
 import java.util.Collections;
@@ -13,6 +11,7 @@
 import de.retest.recheck.ignore.CompoundFilter;
 import de.retest.recheck.ignore.Filter;
 import de.retest.recheck.ignore.Filters;
+import de.retest.recheck.persistence.FileNamer;
 import de.retest.recheck.ui.DefaultValueFinder;
 import de.retest.recheck.ui.Path;
 import de.retest.recheck.ui.descriptors.Element;
@@ -48,9 +47,19 @@ void diff_should_be_created_with_deleted_filtered() {
 	}
 
 	@Test
+	@SuppressWarnings( "deprecation" )
 	void diff_should_handle_legacy_spaces_accordingly() {
-		final FileNamerStrategy fileNamerStrategy = spy( new MavenConformFileNamerStrategy() );
-		doReturn( RecheckImplIT.class.getName() + " legacy spaces" ).when( fileNamerStrategy ).getTestClassName();
+		final FileNamerStrategy fileNamerStrategy = new FileNamerStrategy() {
+			@Override
+			public FileNamer createFileNamer( final String... baseNames ) {
+				return new MavenConformFileNamerStrategy().createFileNamer( baseNames );
+			}
+
+			@Override
+			public String getTestClassName() {
+				return RecheckImplIT.class.getName() + " legacy spaces";
+			}
+		};
 
 		execute( "with legacy spaces", RecheckOptions.builder() //
 				.setIgnore( METADATA_FILTER ) // Ignore metadata

src/test/java/de/retest/recheck/persistence/RecheckTestReportUtilTest.java

@@ -2,17 +2,28 @@
 
 import static de.retest.recheck.RecheckProperties.AGGREGATED_TEST_REPORT_FILE_NAME;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
 
 import java.io.File;
 import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.Collections;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
+import de.retest.recheck.report.ActionReplayResult;
 import de.retest.recheck.report.SuiteReplayResult;
+import de.retest.recheck.report.TestReplayResult;
+import de.retest.recheck.report.action.ActionReplayData;
+import de.retest.recheck.ui.descriptors.Attributes;
+import de.retest.recheck.ui.descriptors.IdentifyingAttributes;
+import de.retest.recheck.ui.descriptors.RootElement;
+import de.retest.recheck.ui.diff.ElementBuilder;
+import de.retest.recheck.ui.diff.ElementBuilder.child2;
+import de.retest.recheck.ui.diff.ElementDifference;
+import de.retest.recheck.ui.diff.RootElementDifference;
+import de.retest.recheck.ui.diff.StateDifference;
 
 class RecheckTestReportUtilTest {
 
@@ -27,8 +38,7 @@ void setUp( @TempDir final Path temp ) {
 
 	@Test
 	void persist_should_create_report_when_there_are_no_diffs() {
-		final SuiteReplayResult replayResult = mock( SuiteReplayResult.class );
-		when( replayResult.getDifferencesCount() ).thenReturn( 0 );
+		final SuiteReplayResult replayResult = new SuiteReplayResult( "test", 23, null, "00", null );
 
 		RecheckTestReportUtil.persist( replayResult, testReport );
 
@@ -37,18 +47,33 @@ void persist_should_create_report_when_there_are_no_diffs() {
 
 	@Test
 	void persist_should_create_report_when_there_are_diffs() {
-		final SuiteReplayResult replayResult = mock( SuiteReplayResult.class );
-		when( replayResult.getDifferencesCount() ).thenReturn( 1 );
+		final SuiteReplayResult replayResult = new SuiteReplayResult( "test", 23, null, "00", null );
+		replayResult.addTest( createDummyReplayResultWithOneDiff() );
 
 		RecheckTestReportUtil.persist( replayResult, testReport );
 
 		assertThat( testReport ).exists();
 	}
 
+	TestReplayResult createDummyReplayResultWithOneDiff() {
+		final RootElement root = new RootElement( "retestId",
+				new IdentifyingAttributes( ElementBuilder
+						.createIdentifyingAttribute( de.retest.recheck.ui.Path.fromString( "comp1" ), child2.class ) ),
+				new Attributes(), null, "screen", 1, "title" );
+		final ActionReplayResult test = ActionReplayResult.createActionReplayResult( ActionReplayData.empty(),
+				new StateDifference( Arrays.asList( new RootElementDifference(
+						new ElementDifference( root, null, null, null, null, Collections.emptyList() ), root,
+						null ) ) ),
+				1, null );
+
+		final TestReplayResult testReplayResult = new TestReplayResult( "test", 1 );
+		testReplayResult.addAction( test );
+		return testReplayResult;
+	}
+
 	@Test
 	void persist_should_create_missing_folders() {
-		final SuiteReplayResult replayResult = mock( SuiteReplayResult.class );
-		when( replayResult.getDifferencesCount() ).thenReturn( 0 );
+		final SuiteReplayResult replayResult = new SuiteReplayResult( "test", 23, null, "00", null );
 
 		RecheckTestReportUtil.persist( replayResult, testReportMissingFolders );
 
@@ -57,7 +82,7 @@ void persist_should_create_missing_folders() {
 
 	@Test
 	void persist_should_create_aggregated_test_report() throws Exception {
-		final SuiteReplayResult replayResult = mock( SuiteReplayResult.class );
+		final SuiteReplayResult replayResult = new SuiteReplayResult( "test", 23, null, "00", null );
 
 		RecheckTestReportUtil.persist( replayResult, testReport );
 

src/test/java/de/retest/recheck/persistence/bin/KryoPersistenceTest.java

@@ -14,7 +14,6 @@
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.ArrayList;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -24,6 +23,7 @@
 
 import de.retest.recheck.persistence.IncompatibleReportVersionException;
 import de.retest.recheck.persistence.Persistable;
+import de.retest.recheck.report.SuiteReplayResult;
 import de.retest.recheck.report.TestReport;
 import de.retest.recheck.util.VersionProvider;
 
@@ -35,18 +35,18 @@ void roundtrip_should_work( @TempDir final Path temp ) throws IOException {
 		Files.createFile( file );
 		final URI identifier = file.toUri();
 
-		final KryoPersistence<de.retest.recheck.test.Test> kryoPersistence = new KryoPersistence<>();
-		final de.retest.recheck.test.Test persisted = createDummyTest();
+		final KryoPersistence<TestReport> kryoPersistence = new KryoPersistence<>();
+		final TestReport persisted = createDummyTest();
 		kryoPersistence.save( identifier, persisted );
-		final de.retest.recheck.test.Test loaded = kryoPersistence.load( identifier );
+		final TestReport loaded = kryoPersistence.load( identifier );
 
-		assertThat( persisted.getRelativeActionSequencePaths() ).isEqualTo( loaded.getRelativeActionSequencePaths() );
+		assertThat( loaded.getSuiteReplayResults().get( 0 ).getName() )
+				.isEqualTo( persisted.getSuiteReplayResults().get( 0 ).getName() );
 	}
 
-	public de.retest.recheck.test.Test createDummyTest() {
-		final ArrayList<String> tests = new ArrayList<>();
-		tests.add( "../test.test" );
-		return new de.retest.recheck.test.Test( tests );
+	public TestReport createDummyTest() {
+		final SuiteReplayResult suite = new SuiteReplayResult( "test", 23, null, "00", null );
+		return new TestReport( suite );
 	}
 
 	@Test
@@ -55,13 +55,12 @@ void incompatible_version_should_give_persisting_version( @TempDir final Path te
 		Files.createFile( file );
 		final URI identifier = file.toUri();
 
-		final KryoPersistence<de.retest.recheck.test.Test> kryoPersistence = new KryoPersistence<>();
+		final KryoPersistence<TestReport> kryoPersistence = new KryoPersistence<>();
 		kryoPersistence.save( identifier, createDummyTest() );
 
 		final Kryo kryoMock = mock( Kryo.class );
 		when( kryoMock.readClassAndObject( any() ) ).thenThrow( KryoException.class );
-		final KryoPersistence<de.retest.recheck.test.Test> differentKryoPersistence =
-				new KryoPersistence<>( kryoMock, "old Version" );
+		final KryoPersistence<TestReport> differentKryoPersistence = new KryoPersistence<>( kryoMock, "old Version" );
 
 		assertThatThrownBy( () -> differentKryoPersistence.load( identifier ) )
 				.isInstanceOf( IncompatibleReportVersionException.class )
@@ -99,8 +98,7 @@ void on_error_file_should_be_deleted() throws IOException {
 		final File nonexistent = new File( "nonexistent.report" );
 		final Kryo kryoMock = mock( Kryo.class );
 		doThrow( KryoException.class ).when( kryoMock ).writeClassAndObject( any(), any() );
-		final KryoPersistence<de.retest.recheck.test.Test> persistence =
-				new KryoPersistence<>( kryoMock, "some version" );
+		final KryoPersistence<TestReport> persistence = new KryoPersistence<>( kryoMock, "some version" );
 		assertThatThrownBy( () -> persistence.save( nonexistent.toURI(), createDummyTest() ) )
 				.isInstanceOf( KryoException.class );
 		assertThat( nonexistent ).doesNotExist();

src/test/java/de/retest/recheck/review/GlobalChangeSetApplierIT.java

@@ -1,13 +1,13 @@
 package de.retest.recheck.review;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
 
 import java.awt.Rectangle;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Random;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -206,7 +206,9 @@ public DefaultValueFinder getDefaultValueFinder() {
 
 		@Override
 		public void notifyAboutDifferences( final ActionReplayResult actionReplayResult ) {
-			final ElementIdentificationWarning warning = mock( ElementIdentificationWarning.class );
+			final ElementIdentificationWarning warning = new ElementIdentificationWarning( "testFileName",
+					new Random().nextInt(), "findByMethodName", "qualifiedTestName" );
+
 			actionReplayResult.getAllElementDifferences().stream() //
 					.map( ElementDifference::getAttributeDifferences ) //
 					.flatMap( Collection::stream ) //

src/test/java/de/retest/recheck/ui/diff/ElementBuilder.java

@@ -18,7 +18,7 @@
 import de.retest.recheck.ui.descriptors.TextAttribute;
 import de.retest.recheck.ui.image.Screenshot;
 
-class ElementBuilder {
+public class ElementBuilder {
 
 	public static Element buildElement() {
 		final IdentifyingAttributes identifyingAttributes =