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/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(); } diff --git a/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java b/src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java index 6ab734d42..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 { @@ -44,9 +46,12 @@ public static GlobalChangeSetApplier create( final TestReport testReport, final // Replay result lookup maps. - private final Multimap, ActionReplayResult> attributeDiffsLookupMap; - private final Multimap insertedDiffsLookupMap; - private final Multimap deletedDiffsLookupMap; + @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 ) { testReport.getSuiteReplayResults().stream() // @@ -70,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 ); } } @@ -80,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 ) { @@ -152,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(); + } } 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; + } + } +}