Skip to content

Commit 2182636

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request #748 from retest/fix/element-warning
Fix/element warning
2 parents 439bb70 + 393ad62 commit 2182636

File tree

5 files changed

+166
-18
lines changed

5 files changed

+166
-18
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ Table of Contents
2727

2828
### Bug Fixes
2929

30+
* Fix accepting of attributes without warning deletes warnings of similar attribute differences.
31+
3032
### New Features
3133

3234
### Improvements

‎src/main/java/de/retest/recheck/review/GlobalChangeSetApplier.java

+41-10
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package de.retest.recheck.review;
22

33
import java.util.Collection;
4+
import java.util.Collections;
45
import java.util.HashMap;
6+
import java.util.HashSet;
7+
import java.util.List;
58
import java.util.Map;
9+
import java.util.Set;
610

711
import org.apache.commons.lang3.tuple.ImmutablePair;
812

@@ -17,8 +21,10 @@
1721
import de.retest.recheck.ui.descriptors.IdentifyingAttributes;
1822
import de.retest.recheck.ui.diff.AttributeDifference;
1923
import de.retest.recheck.ui.diff.ElementDifference;
24+
import de.retest.recheck.ui.diff.ElementIdentificationWarning;
2025
import de.retest.recheck.ui.diff.InsertedDeletedElementDifference;
2126
import de.retest.recheck.ui.review.ActionChangeSet;
27+
import de.retest.recheck.ui.review.AttributeChanges;
2228
import lombok.AccessLevel;
2329
import lombok.Getter;
2430

@@ -29,6 +35,7 @@ public class GlobalChangeSetApplier {
2935
private GlobalChangeSetApplier( final TestReport testReport, final Counter counter ) {
3036
this.counter = counter;
3137
attributeDiffsLookupMap = ArrayListMultimap.create();
38+
warningsLookup = new HashMap<>();
3239
insertedDiffsLookupMap = ArrayListMultimap.create();
3340
deletedDiffsLookupMap = ArrayListMultimap.create();
3441
actionChangeSetLookupMap = new HashMap<>();
@@ -49,6 +56,8 @@ public static GlobalChangeSetApplier create( final TestReport testReport, final
4956
@Getter( AccessLevel.PACKAGE )
5057
private final Multimap<ImmutablePair<String, String>, ActionReplayResult> attributeDiffsLookupMap;
5158
@Getter( AccessLevel.PACKAGE )
59+
private final HashMap<ImmutablePair<String, String>, Set<ElementIdentificationWarning>> warningsLookup;
60+
@Getter( AccessLevel.PACKAGE )
5261
private final Multimap<String, ActionReplayResult> insertedDiffsLookupMap;
5362
@Getter( AccessLevel.PACKAGE )
5463
private final Multimap<String, ActionReplayResult> deletedDiffsLookupMap;
@@ -85,9 +94,13 @@ private void fillAttributeDifferencesLookupMap( final ActionReplayResult actionR
8594
final ElementDifference elementDiff ) {
8695
final IdentifyingAttributes identifyingAttributes = elementDiff.getIdentifyingAttributes();
8796
for ( final AttributeDifference attributeDifference : elementDiff.getAttributeDifferences() ) {
88-
attributeDiffsLookupMap.put(
89-
ImmutablePair.of( identifier( identifyingAttributes ), identifier( attributeDifference ) ),
90-
actionReplayResult );
97+
final ImmutablePair<String, String> key =
98+
ImmutablePair.of( identifier( identifyingAttributes ), identifier( attributeDifference ) );
99+
attributeDiffsLookupMap.put( key, actionReplayResult );
100+
final List<ElementIdentificationWarning> warnings = attributeDifference.getElementIdentificationWarnings();
101+
if ( !warnings.isEmpty() ) {
102+
warningsLookup.computeIfAbsent( key, k -> new HashSet<>() ).addAll( warnings );
103+
}
91104
}
92105
}
93106

@@ -97,6 +110,12 @@ private Collection<ActionReplayResult> findAllActionResultsWithEqualDifferences(
97110
.get( ImmutablePair.of( identifier( identifyingAttributes ), identifier( attributeDifference ) ) );
98111
}
99112

113+
private Set<ElementIdentificationWarning> findAllWarningsForDifference( final IdentifyingAttributes attributes,
114+
final AttributeDifference difference ) {
115+
return warningsLookup.getOrDefault( ImmutablePair.of( identifier( attributes ), identifier( difference ) ),
116+
Collections.emptySet() );
117+
}
118+
100119
private ActionChangeSet findCorrespondingActionChangeSet( final ActionReplayResult actionReplayResult ) {
101120
final ActionChangeSet actionChangeSet = actionChangeSetLookupMap.get( actionReplayResult );
102121
assert actionChangeSet != null : "Error, introduce() wasn't called for this actionReplayResult!";
@@ -121,7 +140,8 @@ public void addChangeSetForAllEqualIdentAttributeChanges( final IdentifyingAttri
121140
for ( final ActionReplayResult actionReplayResult : actionResultsWithDiffs ) {
122141
final ActionChangeSet correspondingActionChangeSet = findCorrespondingActionChangeSet( actionReplayResult );
123142
assert correspondingActionChangeSet != null : "Should have been added during load and thus not be empty!";
124-
correspondingActionChangeSet.getIdentAttributeChanges().add( identifyingAttributes, attributeDifference );
143+
final AttributeChanges changes = correspondingActionChangeSet.getIdentAttributeChanges();
144+
changes.add( identifyingAttributes, injectWarningsFor( identifyingAttributes, attributeDifference ) );
125145
}
126146
counter.add();
127147
}
@@ -130,8 +150,9 @@ public void createChangeSetForAllEqualAttributesChanges( final IdentifyingAttrib
130150
final AttributeDifference attributeDifference ) {
131151
for ( final ActionReplayResult actionReplayResult : findAllActionResultsWithEqualDifferences(
132152
identifyingAttributes, attributeDifference ) ) {
133-
findCorrespondingActionChangeSet( actionReplayResult ).getAttributesChanges().add( identifyingAttributes,
134-
attributeDifference );
153+
final ActionChangeSet correspondingActionChangeSet = findCorrespondingActionChangeSet( actionReplayResult );
154+
final AttributeChanges changes = correspondingActionChangeSet.getAttributesChanges();
155+
changes.add( identifyingAttributes, injectWarningsFor( identifyingAttributes, attributeDifference ) );
135156
}
136157
counter.add();
137158
}
@@ -140,8 +161,9 @@ public void removeChangeSetForAllEqualIdentAttributeChanges( final IdentifyingAt
140161
final AttributeDifference attributeDifference ) {
141162
for ( final ActionReplayResult actionReplayResult : findAllActionResultsWithEqualDifferences(
142163
identifyingAttributes, attributeDifference ) ) {
143-
findCorrespondingActionChangeSet( actionReplayResult ).getIdentAttributeChanges()
144-
.remove( identifyingAttributes, attributeDifference );
164+
final ActionChangeSet correspondingActionChangeSet = findCorrespondingActionChangeSet( actionReplayResult );
165+
final AttributeChanges changes = correspondingActionChangeSet.getIdentAttributeChanges();
166+
changes.remove( identifyingAttributes, injectWarningsFor( identifyingAttributes, attributeDifference ) );
145167
}
146168
counter.remove();
147169
}
@@ -150,12 +172,21 @@ public void removeChangeSetForAllEqualAttributesChanges( final IdentifyingAttrib
150172
final AttributeDifference attributeDifference ) {
151173
for ( final ActionReplayResult actionReplayResult : findAllActionResultsWithEqualDifferences(
152174
identifyingAttributes, attributeDifference ) ) {
153-
findCorrespondingActionChangeSet( actionReplayResult ).getAttributesChanges().remove( identifyingAttributes,
154-
attributeDifference );
175+
final ActionChangeSet correspondingActionChangeSet = findCorrespondingActionChangeSet( actionReplayResult );
176+
final AttributeChanges changes = correspondingActionChangeSet.getAttributesChanges();
177+
changes.remove( identifyingAttributes, injectWarningsFor( identifyingAttributes, attributeDifference ) );
155178
}
156179
counter.remove();
157180
}
158181

182+
private AttributeDifference injectWarningsFor( final IdentifyingAttributes attributes,
183+
final AttributeDifference difference ) {
184+
final AttributeDifference copy =
185+
new AttributeDifference( difference.getKey(), difference.getExpected(), difference.getActual() );
186+
copy.addElementIdentificationWarnings( findAllWarningsForDifference( attributes, difference ) );
187+
return copy;
188+
}
189+
159190
// Add/remove inserted/deleted differences.
160191

161192
public void addChangeSetForAllEqualInsertedChanges( final Element inserted ) {

‎src/main/java/de/retest/recheck/ui/diff/AttributeDifference.java

+5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import java.io.Serializable;
77
import java.util.ArrayList;
8+
import java.util.Collection;
89
import java.util.Collections;
910
import java.util.List;
1011
import java.util.Objects;
@@ -80,6 +81,10 @@ public void addElementIdentificationWarning( final ElementIdentificationWarning
8081
elementIdentificationWarnings.add( elementIdentificationWarning );
8182
}
8283

84+
public void addElementIdentificationWarnings( final Collection<? extends ElementIdentificationWarning> warnings ) {
85+
elementIdentificationWarnings.addAll( warnings );
86+
}
87+
8388
public boolean hasElementIdentificationWarning() {
8489
return !elementIdentificationWarnings.isEmpty();
8590
}

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

+27
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package de.retest.recheck.review;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.mockito.Mockito.mock;
45

56
import java.awt.Rectangle;
67
import java.nio.file.Files;
78
import java.nio.file.Path;
9+
import java.util.Collection;
810
import java.util.Collections;
911
import java.util.Set;
12+
import java.util.function.Function;
1013
import java.util.stream.Collectors;
1114
import java.util.stream.Stream;
1215

@@ -20,6 +23,7 @@
2023
import de.retest.recheck.RecheckOptions;
2124
import de.retest.recheck.SuiteAggregator;
2225
import de.retest.recheck.persistence.SeparatePathsProjectLayout;
26+
import de.retest.recheck.report.ActionReplayResult;
2327
import de.retest.recheck.report.TestReport;
2428
import de.retest.recheck.ui.DefaultValueFinder;
2529
import de.retest.recheck.ui.PathElement;
@@ -31,6 +35,9 @@
3135
import de.retest.recheck.ui.descriptors.OutlineAttribute;
3236
import de.retest.recheck.ui.descriptors.RootElement;
3337
import de.retest.recheck.ui.descriptors.StringAttribute;
38+
import de.retest.recheck.ui.diff.ElementDifference;
39+
import de.retest.recheck.ui.diff.ElementIdentificationWarning;
40+
import lombok.extern.slf4j.Slf4j;
3441

3542
class GlobalChangeSetApplierIT {
3643

@@ -93,6 +100,12 @@ void lookups_should_ignore_outline_changes() {
93100
assertThat( cut.getDeletedDiffsLookupMap().asMap() ).hasSize( 1 );
94101
}
95102

103+
@Test
104+
void lookups_should_contain_all_warnings() {
105+
// There are two differences for align-self (large, small)
106+
assertThat( cut.getWarningsLookup().values() ).flatExtracting( Function.identity() ).hasSize( 2 );
107+
}
108+
96109
private void capTest( final Recheck re ) {
97110
try {
98111
re.capTest();
@@ -173,6 +186,7 @@ private Attributes attributes( final String... entries ) {
173186
return attributes.immutable();
174187
}
175188

189+
@Slf4j
176190
private static class RootElementRecheckAdapter implements RecheckAdapter {
177191

178192
@Override
@@ -189,5 +203,18 @@ public Set<RootElement> convert( final Object toCheck ) {
189203
public DefaultValueFinder getDefaultValueFinder() {
190204
return ( identifyingAttributes, attributeKey, attributeValue ) -> false;
191205
}
206+
207+
@Override
208+
public void notifyAboutDifferences( final ActionReplayResult actionReplayResult ) {
209+
final ElementIdentificationWarning warning = mock( ElementIdentificationWarning.class );
210+
actionReplayResult.getAllElementDifferences().stream() //
211+
.map( ElementDifference::getAttributeDifferences ) //
212+
.flatMap( Collection::stream ) //
213+
.filter( difference -> "align-self".equals( difference.getKey() ) ) //
214+
.forEach( difference -> {
215+
log.debug( "Adding warning to difference '{}' for '{}'.", difference, actionReplayResult );
216+
difference.addElementIdentificationWarning( warning );
217+
} );
218+
}
192219
}
193220
}

‎src/test/java/de/retest/recheck/review/GlobalChangeSetApplierTest.java

+91-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
package de.retest.recheck.review;
22

3+
import static org.assertj.core.api.Assertions.assertThat;
34
import static org.junit.jupiter.api.Assertions.assertThrows;
5+
import static org.mockito.ArgumentMatchers.any;
46
import static org.mockito.ArgumentMatchers.anyString;
7+
import static org.mockito.ArgumentMatchers.eq;
58
import static org.mockito.Mockito.atLeastOnce;
69
import static org.mockito.Mockito.mock;
710
import static org.mockito.Mockito.only;
@@ -11,10 +14,12 @@
1114
import static org.mockito.Mockito.when;
1215

1316
import java.util.Arrays;
17+
import java.util.Collections;
1418
import java.util.List;
1519

1620
import org.junit.jupiter.api.BeforeEach;
1721
import org.junit.jupiter.api.Test;
22+
import org.mockito.ArgumentCaptor;
1823

1924
import de.retest.recheck.report.ActionReplayResult;
2025
import de.retest.recheck.report.SuiteReplayResult;
@@ -24,6 +29,7 @@
2429
import de.retest.recheck.ui.descriptors.IdentifyingAttributes;
2530
import de.retest.recheck.ui.diff.AttributeDifference;
2631
import de.retest.recheck.ui.diff.ElementDifference;
32+
import de.retest.recheck.ui.diff.ElementIdentificationWarning;
2733
import de.retest.recheck.ui.diff.InsertedDeletedElementDifference;
2834
import de.retest.recheck.ui.review.ActionChangeSet;
2935
import de.retest.recheck.ui.review.AttributeChanges;
@@ -65,7 +71,11 @@ class GlobalChangeSetApplierTest {
6571
@BeforeEach
6672
void setUp() {
6773
identifyingAttributes = mock( IdentifyingAttributes.class );
74+
when( identifyingAttributes.identifier() ).thenReturn( "a" );
6875
attributeDifference = mock( AttributeDifference.class );
76+
when( attributeDifference.identifier() ).thenReturn( "a" );
77+
when( attributeDifference.getElementIdentificationWarnings() )
78+
.thenReturn( Collections.singletonList( mock( ElementIdentificationWarning.class ) ) );
6979

7080
testReport = mock( TestReport.class );
7181
suiteReplayResult = mock( SuiteReplayResult.class );
@@ -143,6 +153,7 @@ void create_should_read_all_elements_from_replayResult() {
143153
verify( elementDifference1, times( 1 ) ).getIdentifyingAttributes();
144154
verify( elementDifference1, times( 1 ) ).isInsertionOrDeletion();
145155
verifyNoMoreInteractions( elementDifference1 );
156+
verify( attributeDifference, times( 2 ) ).getElementIdentificationWarnings();
146157
}
147158

148159
// Add/remove element differences.
@@ -154,8 +165,26 @@ void add_for_all_ident_should_add_ident_change_set() throws Exception {
154165

155166
globalApplier.addChangeSetForAllEqualIdentAttributeChanges( identifyingAttributes, attributeDifference );
156167

157-
verify( identifyingAttributesChangeSet1, times( 1 ) ).add( identifyingAttributes, attributeDifference );
158-
verify( identifyingAttributesChangeSet2, times( 1 ) ).add( identifyingAttributes, attributeDifference );
168+
verify( identifyingAttributesChangeSet1, times( 1 ) ).add( eq( identifyingAttributes ), any() );
169+
verify( identifyingAttributesChangeSet2, times( 1 ) ).add( eq( identifyingAttributes ), any() );
170+
}
171+
172+
@Test
173+
void add_for_all_ident_without_warnings_should_inject_warnings() {
174+
globalApplier.introduce( actionReplayResult1, actionChangeSet1 );
175+
globalApplier.introduce( actionReplayResult2, actionChangeSet2 );
176+
177+
final AttributeDifference withoutWarnings = mock( AttributeDifference.class );
178+
when( withoutWarnings.identifier() ).thenReturn( "a" );
179+
180+
globalApplier.addChangeSetForAllEqualIdentAttributeChanges( identifyingAttributes, withoutWarnings );
181+
182+
final ArgumentCaptor<AttributeDifference> captor = ArgumentCaptor.forClass( AttributeDifference.class );
183+
verify( identifyingAttributesChangeSet1, times( 1 ) ).add( any(), captor.capture() );
184+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
185+
186+
verify( identifyingAttributesChangeSet2, times( 1 ) ).add( any(), captor.capture() );
187+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
159188
}
160189

161190
@Test
@@ -165,8 +194,26 @@ void add_for_all_state_should_add_state_change_set() throws Exception {
165194

166195
globalApplier.createChangeSetForAllEqualAttributesChanges( identifyingAttributes, attributeDifference );
167196

168-
verify( attributeChangeSet1, times( 1 ) ).add( identifyingAttributes, attributeDifference );
169-
verify( attributeChangeSet2, times( 1 ) ).add( identifyingAttributes, attributeDifference );
197+
verify( attributeChangeSet1, times( 1 ) ).add( eq( identifyingAttributes ), any() );
198+
verify( attributeChangeSet2, times( 1 ) ).add( eq( identifyingAttributes ), any() );
199+
}
200+
201+
@Test
202+
void add_for_all_state_without_warnings_should_inject_warnings() {
203+
globalApplier.introduce( actionReplayResult1, actionChangeSet1 );
204+
globalApplier.introduce( actionReplayResult2, actionChangeSet2 );
205+
206+
final AttributeDifference withoutWarnings = mock( AttributeDifference.class );
207+
when( withoutWarnings.identifier() ).thenReturn( "a" );
208+
209+
globalApplier.createChangeSetForAllEqualAttributesChanges( identifyingAttributes, withoutWarnings );
210+
211+
final ArgumentCaptor<AttributeDifference> captor = ArgumentCaptor.forClass( AttributeDifference.class );
212+
verify( attributeChangeSet1, times( 1 ) ).add( any(), captor.capture() );
213+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
214+
215+
verify( attributeChangeSet2, times( 1 ) ).add( any(), captor.capture() );
216+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
170217
}
171218

172219
@Test
@@ -176,8 +223,26 @@ void remove_for_all_ident_should_remove_ident_change_set() throws Exception {
176223

177224
globalApplier.removeChangeSetForAllEqualIdentAttributeChanges( identifyingAttributes, attributeDifference );
178225

179-
verify( identifyingAttributesChangeSet1, times( 1 ) ).remove( identifyingAttributes, attributeDifference );
180-
verify( identifyingAttributesChangeSet2, times( 1 ) ).remove( identifyingAttributes, attributeDifference );
226+
verify( identifyingAttributesChangeSet1, times( 1 ) ).remove( eq( identifyingAttributes ), any() );
227+
verify( identifyingAttributesChangeSet2, times( 1 ) ).remove( eq( identifyingAttributes ), any() );
228+
}
229+
230+
@Test
231+
void remove_for_all_ident_should_inject_warnings() {
232+
globalApplier.introduce( actionReplayResult1, actionChangeSet1 );
233+
globalApplier.introduce( actionReplayResult2, actionChangeSet2 );
234+
235+
final AttributeDifference withoutWarnings = mock( AttributeDifference.class );
236+
when( withoutWarnings.identifier() ).thenReturn( "a" );
237+
238+
globalApplier.removeChangeSetForAllEqualIdentAttributeChanges( identifyingAttributes, withoutWarnings );
239+
240+
final ArgumentCaptor<AttributeDifference> captor = ArgumentCaptor.forClass( AttributeDifference.class );
241+
verify( identifyingAttributesChangeSet1, times( 1 ) ).remove( any(), captor.capture() );
242+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
243+
244+
verify( identifyingAttributesChangeSet2, times( 1 ) ).remove( any(), captor.capture() );
245+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
181246
}
182247

183248
@Test
@@ -187,8 +252,26 @@ void remove_for_all_state_should_remove_state_change_set() throws Exception {
187252

188253
globalApplier.removeChangeSetForAllEqualAttributesChanges( identifyingAttributes, attributeDifference );
189254

190-
verify( attributeChangeSet1, atLeastOnce() ).remove( identifyingAttributes, attributeDifference );
191-
verify( attributeChangeSet2, atLeastOnce() ).remove( identifyingAttributes, attributeDifference );
255+
verify( attributeChangeSet1, atLeastOnce() ).remove( eq( identifyingAttributes ), any() );
256+
verify( attributeChangeSet2, atLeastOnce() ).remove( eq( identifyingAttributes ), any() );
257+
}
258+
259+
@Test
260+
void remove_for_all_state_should_inject_warnings() {
261+
globalApplier.introduce( actionReplayResult1, actionChangeSet1 );
262+
globalApplier.introduce( actionReplayResult2, actionChangeSet2 );
263+
264+
final AttributeDifference withoutWarnings = mock( AttributeDifference.class );
265+
when( withoutWarnings.identifier() ).thenReturn( "a" );
266+
267+
globalApplier.removeChangeSetForAllEqualAttributesChanges( identifyingAttributes, withoutWarnings );
268+
269+
final ArgumentCaptor<AttributeDifference> captor = ArgumentCaptor.forClass( AttributeDifference.class );
270+
verify( attributeChangeSet1, times( 1 ) ).remove( any(), captor.capture() );
271+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
272+
273+
verify( attributeChangeSet2, times( 1 ) ).remove( any(), captor.capture() );
274+
assertThat( captor.getValue().getElementIdentificationWarnings() ).hasSize( 1 );
192275
}
193276

194277
@Test

0 commit comments

Comments
 (0)