From f300ba45ecbe461474eb5319167ecae60aef4b59 Mon Sep 17 00:00:00 2001 From: diba1013 Date: Mon, 11 May 2020 11:09:55 +0200 Subject: [PATCH 1/3] Add reset to test to ensure clean working state since the TestReport is shared between all tests within the VM --- src/main/java/de/retest/recheck/SuiteAggregator.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/main/java/de/retest/recheck/SuiteAggregator.java b/src/main/java/de/retest/recheck/SuiteAggregator.java index 36bc94244..3111ef5f9 100644 --- a/src/main/java/de/retest/recheck/SuiteAggregator.java +++ b/src/main/java/de/retest/recheck/SuiteAggregator.java @@ -19,6 +19,13 @@ public static SuiteAggregator getInstance() { return instance; } + /** + * Must only be called from a test + */ + public static void reset() { + instance = null; + } + static SuiteAggregator getTestInstance() { return new SuiteAggregator(); } From 8b9992f97fd1b614cde2bede6eb6f6acd4b9c800 Mon Sep 17 00:00:00 2001 From: diba1013 Date: Mon, 11 May 2020 10:54:44 +0200 Subject: [PATCH 2/3] Add failing test to outline the problem which emulates the following use case: 1. Test the same website with two different resolutions. 2. Create initial Golden Master. 3. Compare with slight resolution differences, css differences and removed element. Since the same elements are compared (only different resolution and thus different outline), the GlobalChangeSetApplier should still merge the same elements. --- .../review/GlobalChangeSetApplier.java | 3 + .../review/GlobalChangeSetApplierIT.java | 193 ++++++++++++++++++ 2 files changed, 196 insertions(+) create mode 100644 src/test/java/de/retest/recheck/review/GlobalChangeSetApplierIT.java diff --git a/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java b/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java index 6ab734d42..183aa0c3c 100644 --- a/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java +++ b/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java @@ -44,8 +44,11 @@ public static GlobalChangeSetApplier create( final TestReport testReport, final // Replay result lookup maps. + @Getter( AccessLevel.PACKAGE ) private final Multimap, ActionReplayResult> attributeDiffsLookupMap; + @Getter( AccessLevel.PACKAGE ) private final Multimap insertedDiffsLookupMap; + @Getter( AccessLevel.PACKAGE ) private final Multimap deletedDiffsLookupMap; private void fillReplayResultLookupMaps( final TestReport testReport ) { diff --git a/src/test/java/de/retest/recheck/review/GlobalChangeSetApplierIT.java b/src/test/java/de/retest/recheck/review/GlobalChangeSetApplierIT.java new file mode 100644 index 000000000..aca5240fd --- /dev/null +++ b/src/test/java/de/retest/recheck/review/GlobalChangeSetApplierIT.java @@ -0,0 +1,193 @@ +package de.retest.recheck.review; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.awt.Rectangle; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import de.retest.recheck.Recheck; +import de.retest.recheck.RecheckAdapter; +import de.retest.recheck.RecheckImpl; +import de.retest.recheck.RecheckOptions; +import de.retest.recheck.SuiteAggregator; +import de.retest.recheck.persistence.SeparatePathsProjectLayout; +import de.retest.recheck.report.TestReport; +import de.retest.recheck.ui.DefaultValueFinder; +import de.retest.recheck.ui.PathElement; +import de.retest.recheck.ui.descriptors.Attribute; +import de.retest.recheck.ui.descriptors.Attributes; +import de.retest.recheck.ui.descriptors.Element; +import de.retest.recheck.ui.descriptors.IdentifyingAttributes; +import de.retest.recheck.ui.descriptors.MutableAttributes; +import de.retest.recheck.ui.descriptors.OutlineAttribute; +import de.retest.recheck.ui.descriptors.RootElement; +import de.retest.recheck.ui.descriptors.StringAttribute; + +class GlobalChangeSetApplierIT { + + GlobalChangeSetApplier cut; + + @BeforeEach + void setUp( @TempDir final Path root ) throws Exception { + SuiteAggregator.reset(); + + Files.createDirectory( root.resolve( ".retest" ) ); + + final RecheckAdapter adapter = new RootElementRecheckAdapter(); + + final Recheck re = new RecheckImpl( RecheckOptions.builder() // + .suiteName( "TemplateTest" ) // + .projectLayout( new SeparatePathsProjectLayout( root.resolve( "states" ), root.resolve( "reports" ) ) ) // + .build() ); + + // Create first step to create the Golden Masters + + re.startTest( "large" ); + re.check( expected( 1200, 800 ), adapter, "page" ); + capTest( re ); + + re.startTest( "small" ); + re.check( expected( 800, 400 ), adapter, "page" ); + capTest( re ); + + // Create second step with slight pixel differences to force the identifying attributes to contain differences + + re.startTest( "large" ); + re.check( actual( 1190, 795 ), adapter, "page" ); + capTest( re ); + + re.startTest( "small" ); + re.check( actual( 750, 390 ), adapter, "page" ); + capTest( re ); + + re.cap(); + + final TestReport report = SuiteAggregator.getInstance().getAggregatedTestReport(); + cut = GlobalChangeSetApplier.create( report ); + } + + @Test + void lookups_should_ignore_outline_changes() { + // Differences: + // html[1]/body[1]/div[1] + // - outline: expected="[0,0,300,200]", actual="[0,0,297,198]" (large) + // - outline: expected="[0,0,200,100]", actual="[0,0,187,97]" (small) + // - align-self: expected="", actual="flex-start" (large, small) + assertThat( cut.getAttributeDiffsLookupMap().asMap() ).hasSize( 3 ); + + // Inserted: + // html[1] (large, small) + assertThat( cut.getInsertedDiffsLookupMap().asMap() ).hasSize( 1 ); + + // Deleted: + // html[1]/body[1]/span[1] (large, small) + assertThat( cut.getDeletedDiffsLookupMap().asMap() ).hasSize( 1 ); + } + + private void capTest( final Recheck re ) { + try { + re.capTest(); + } catch ( final AssertionError error ) { + // Ignore + } + } + + private RootElement expected( final int width, final int height ) { + final RootElement html = html(); + final Element body = body( html ); + final Element div = div( body, 1, width, height, attributes( "align-self", "" ) ); + final Element span = span( body, 1, width, height, new Attributes() ); + + html.addChildren( body ); + body.addChildren( div, span ); + + return html; + } + + private RootElement actual( final int width, final int height ) { + final RootElement html = html(); + final Element body = body( html ); + final Element div = div( body, 1, width, height, attributes( "align-self", "flex-start" ) ); + + html.addChildren( body ); + body.addChildren( div ); + + return html; + } + + private RootElement html() { + return new RootElement( "html", id( path( "html", 1 ) ), new Attributes(), null, "screen", 0, "title" ); + } + + private Element body( final Element parent ) { + return Element.create( "div", parent, id( path( parent, "body", 1 ) ), new Attributes() ); + } + + private Element div( final Element parent, final int suffix, final int width, final int height, + final Attributes attributes ) { + return Element.create( "div", parent, id( path( parent, "div", suffix ), // + OutlineAttribute.createAbsolute( new Rectangle( width / 2, height / 2, width / 4, height / 4 ) ), // + OutlineAttribute.create( new Rectangle( 0, 0, width / 4, height / 4 ) ), // + new StringAttribute( "class", "md-sidebar md-sidebar--primary" ) // + ), attributes ); + } + + private Element span( final Element parent, final int suffix, final int width, final int height, + final Attributes attributes ) { + return Element.create( "span", parent, id( path( parent, "span", suffix ), // + OutlineAttribute.createAbsolute( new Rectangle( 0, height / 2, width / 2, height / 4 ) ), // + OutlineAttribute.create( new Rectangle( 0, 0, width / 4, height / 4 ) ) // + ), attributes ); + } + + private IdentifyingAttributes id( final de.retest.recheck.ui.Path path, final Attribute... attributes ) { + return new IdentifyingAttributes( Stream.concat( // + IdentifyingAttributes.createList( path, path.getElement().getElementName() ).stream(), // + Stream.of( attributes ) // + ).collect( Collectors.toList() ) ); + } + + private de.retest.recheck.ui.Path path( final String type, final int suffix ) { + return de.retest.recheck.ui.Path.path( new PathElement( type, suffix ) ); + } + + private de.retest.recheck.ui.Path path( final Element parent, final String type, final int suffix ) { + return de.retest.recheck.ui.Path.path( parent.getIdentifyingAttributes().getPathTyped(), + new PathElement( type, suffix ) ); + } + + private Attributes attributes( final String... entries ) { + final MutableAttributes attributes = new MutableAttributes(); + for ( int i = 0; i < entries.length; i += 2 ) { + attributes.put( entries[i], entries[i + 1] ); + } + return attributes.immutable(); + } + + private static class RootElementRecheckAdapter implements RecheckAdapter { + + @Override + public boolean canCheck( final Object toCheck ) { + return toCheck instanceof RootElement; + } + + @Override + public Set convert( final Object toCheck ) { + return Collections.singleton( (RootElement) toCheck ); + } + + @Override + public DefaultValueFinder getDefaultValueFinder() { + return ( identifyingAttributes, attributeKey, attributeValue ) -> false; + } + } +} From f7e992a738ee8c9f1935b284eac14424f07349b8 Mon Sep 17 00:00:00 2001 From: diba1013 Date: Mon, 11 May 2020 11:02:19 +0200 Subject: [PATCH 3/3] Use identifier methods to reduce noise since the outline is not a stable attribute and thus falsifies the hashCode of IdentifyingAttributes. The same goes for the AttributeDifference, which may or may not contain warnings. --- CHANGELOG.md | 1 + .../review/GlobalChangeSetApplier.java | 37 +++++++++++++------ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60d21199d..e3cf155e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ Table of Contents ### Bug Fixes * Fixed a java.lang.ClassCastException: de.retest.recheck.ui.diff.InsertedDeletedElementDifference cannot be cast to de.retest.recheck.ui.diff.IdentifyingAttributesDifference. +* Fix similar differences between checks not being accepted together, bringing back 1-click-maintenance. ### New Features diff --git a/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java b/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java index 183aa0c3c..0fa400b8c 100644 --- a/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java +++ b/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java @@ -19,6 +19,8 @@ import de.retest.recheck.ui.diff.ElementDifference; import de.retest.recheck.ui.diff.InsertedDeletedElementDifference; import de.retest.recheck.ui.review.ActionChangeSet; +import lombok.AccessLevel; +import lombok.Getter; public class GlobalChangeSetApplier { @@ -45,11 +47,11 @@ public static GlobalChangeSetApplier create( final TestReport testReport, final // Replay result lookup maps. @Getter( AccessLevel.PACKAGE ) - private final Multimap, ActionReplayResult> attributeDiffsLookupMap; + private final Multimap, ActionReplayResult> attributeDiffsLookupMap; @Getter( AccessLevel.PACKAGE ) - private final Multimap insertedDiffsLookupMap; + private final Multimap insertedDiffsLookupMap; @Getter( AccessLevel.PACKAGE ) - private final Multimap deletedDiffsLookupMap; + private final Multimap deletedDiffsLookupMap; private void fillReplayResultLookupMaps( final TestReport testReport ) { testReport.getSuiteReplayResults().stream() // @@ -73,9 +75,9 @@ private void fillInsertedDeletedDifferencesLookupMaps( final ActionReplayResult final InsertedDeletedElementDifference insertedDeletedElementDiff = (InsertedDeletedElementDifference) elementDiff.getIdentifyingAttributesDifference(); if ( insertedDeletedElementDiff.isInserted() ) { - insertedDiffsLookupMap.put( insertedDeletedElementDiff.getActual(), actionReplayResult ); + insertedDiffsLookupMap.put( identifier( insertedDeletedElementDiff.getActual() ), actionReplayResult ); } else { - deletedDiffsLookupMap.put( elementDiff.getIdentifyingAttributes(), actionReplayResult ); + deletedDiffsLookupMap.put( identifier( elementDiff.getIdentifyingAttributes() ), actionReplayResult ); } } @@ -83,14 +85,16 @@ private void fillAttributeDifferencesLookupMap( final ActionReplayResult actionR final ElementDifference elementDiff ) { final IdentifyingAttributes identifyingAttributes = elementDiff.getIdentifyingAttributes(); for ( final AttributeDifference attributeDifference : elementDiff.getAttributeDifferences() ) { - attributeDiffsLookupMap.put( ImmutablePair.of( identifyingAttributes, attributeDifference ), + attributeDiffsLookupMap.put( + ImmutablePair.of( identifier( identifyingAttributes ), identifier( attributeDifference ) ), actionReplayResult ); } } private Collection findAllActionResultsWithEqualDifferences( final IdentifyingAttributes identifyingAttributes, final AttributeDifference attributeDifference ) { - return attributeDiffsLookupMap.get( ImmutablePair.of( identifyingAttributes, attributeDifference ) ); + return attributeDiffsLookupMap + .get( ImmutablePair.of( identifier( identifyingAttributes ), identifier( attributeDifference ) ) ); } private ActionChangeSet findCorrespondingActionChangeSet( final ActionReplayResult actionReplayResult ) { @@ -155,31 +159,42 @@ public void removeChangeSetForAllEqualAttributesChanges( final IdentifyingAttrib // Add/remove inserted/deleted differences. public void addChangeSetForAllEqualInsertedChanges( final Element inserted ) { - for ( final ActionReplayResult replayResult : insertedDiffsLookupMap.get( inserted ) ) { + for ( final ActionReplayResult replayResult : insertedDiffsLookupMap.get( identifier( inserted ) ) ) { findCorrespondingActionChangeSet( replayResult ).addInsertChange( inserted ); } counter.add(); } public void addChangeSetForAllEqualDeletedChanges( final IdentifyingAttributes deleted ) { - for ( final ActionReplayResult replayResult : deletedDiffsLookupMap.get( deleted ) ) { + for ( final ActionReplayResult replayResult : deletedDiffsLookupMap.get( identifier( deleted ) ) ) { findCorrespondingActionChangeSet( replayResult ).addDeletedChange( deleted ); } counter.add(); } public void removeChangeSetForAllEqualInsertedChanges( final Element inserted ) { - for ( final ActionReplayResult replayResult : insertedDiffsLookupMap.get( inserted ) ) { + for ( final ActionReplayResult replayResult : insertedDiffsLookupMap.get( identifier( inserted ) ) ) { findCorrespondingActionChangeSet( replayResult ).removeInsertChange( inserted ); } counter.remove(); } public void removeChangeSetForAllEqualDeletedChanges( final IdentifyingAttributes deleted ) { - for ( final ActionReplayResult replayResult : deletedDiffsLookupMap.get( deleted ) ) { + for ( final ActionReplayResult replayResult : deletedDiffsLookupMap.get( identifier( deleted ) ) ) { findCorrespondingActionChangeSet( replayResult ).removeDeletedChange( deleted ); } counter.remove(); } + private String identifier( final Element element ) { + return identifier( element.getIdentifyingAttributes() ); + } + + private String identifier( final IdentifyingAttributes attributes ) { + return attributes.identifier(); + } + + private String identifier( final AttributeDifference difference ) { + return difference.identifier(); + } }