You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/10/28 01:28:53 UTC
[GitHub] [commons-collections] anantdahiya8 opened a new pull request, #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
anantdahiya8 opened a new pull request, #353:
URL: https://github.com/apache/commons-collections/pull/353
In test : `org.apache.commons.collections4.bidimap.DualHashBidiMapTest.testBidiKeySetValuesOrder`
The order of keyset and valueSet can differ because it uses `HashMap` which is non deterministic so its order cannot be determined.
Replace `HashMap' with 'LinkedHashMap' so the order of keySet and valueSet remains same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] kinow commented on a diff in pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #353:
URL: https://github.com/apache/commons-collections/pull/353#discussion_r1007547336
##########
src/main/java/org/apache/commons/collections4/bidimap/DualHashBidiMap.java:
##########
@@ -22,6 +22,7 @@
import java.io.Serializable;
import java.util.HashMap;
import java.util.Map;
+import java.util.LinkedMap;
Review Comment:
`s/LinkedMap/LinkedHashMap`
Make sure you read `CONTRIBUTING.md` too as it contains useful information for contributing code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] aherbert commented on pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
aherbert commented on PR #353:
URL: https://github.com/apache/commons-collections/pull/353#issuecomment-1303300681
This is failing CI due to a configuration error in the POM that somehow brings in the wrong junit runner for this junit5 test and the TestAbortedException causes an error and not an ignored test. Since we have no junit4 test fixtures I have updated master to remove junit-jupiter-vintage.
If you rebase and push this should work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] anantdahiya8 commented on pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
anantdahiya8 commented on PR #353:
URL: https://github.com/apache/commons-collections/pull/353#issuecomment-1294684766
>
Yup, this is what I was thinking. Thanks for the reference, will make the change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] codecov-commenter commented on pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #353:
URL: https://github.com/apache/commons-collections/pull/353#issuecomment-1294395441
# [Codecov](https://codecov.io/gh/apache/commons-collections/pull/353?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#353](https://codecov.io/gh/apache/commons-collections/pull/353?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8a4e2f6) into [master](https://codecov.io/gh/apache/commons-collections/commit/a8688157322ea0a8a9c45df51acf8c679a9557e1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a868815) will **not change** coverage.
> The diff coverage is `50.00%`.
```diff
@@ Coverage Diff @@
## master #353 +/- ##
=========================================
Coverage 85.98% 85.98%
Complexity 4671 4671
=========================================
Files 289 289
Lines 13447 13447
Branches 1977 1977
=========================================
Hits 11563 11563
Misses 1323 1323
Partials 561 561
```
| [Impacted Files](https://codecov.io/gh/apache/commons-collections/pull/353?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [.../commons/collections4/bidimap/DualHashBidiMap.java](https://codecov.io/gh/apache/commons-collections/pull/353/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvY29sbGVjdGlvbnM0L2JpZGltYXAvRHVhbEhhc2hCaWRpTWFwLmphdmE=) | `82.35% <50.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] anantdahiya8 commented on pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
anantdahiya8 commented on PR #353:
URL: https://github.com/apache/commons-collections/pull/353#issuecomment-1301682947
@aherbert Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] aherbert commented on pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
aherbert commented on PR #353:
URL: https://github.com/apache/commons-collections/pull/353#issuecomment-1294562168
Note that there already is a `DualLinkedHashBidiMap` object. So this change is converting the `DualHashBidiMap` to a `DualLinkedHashBidiMap`. This is not the solution.
-1
As with previous PRs related to a similar issue, we should change the test rather than the implementation. See #341, #336.
If the iteration order of the DualHashBidiMap is not specified, then do not change it to pass an arbitrary test. To determine how flaky this test is you could try using the `nondex` plugin. See the other linked PRs for examples.
A simple fix can be to change `testBidiKeySetValuesOrder` to check the `getIterationBehaviour()` flag and skip this test:
```Java
@Test
public void testBidiKeySetValuesOrder() {
Assume.assumeTrue((getIterationBehaviour() & AbstractCollectionTest.UNORDERED) == 0);
...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] anantdahiya8 commented on a diff in pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
anantdahiya8 commented on code in PR #353:
URL: https://github.com/apache/commons-collections/pull/353#discussion_r1013557830
##########
src/test/java/org/apache/commons/collections4/bidimap/AbstractBidiMapTest.java:
##########
@@ -265,16 +266,18 @@ private void removeValue(final BidiMap<?, ?> map, final Object value) {
@Test
public void testBidiKeySetValuesOrder() {
- resetFull();
- final Iterator<K> keys = map.keySet().iterator();
- final Iterator<V> values = map.values().iterator();
- while (keys.hasNext() && values.hasNext()) {
- final K key = keys.next();
- final V value = values.next();
- assertSame(map.get(key), value);
- }
- assertFalse(keys.hasNext());
- assertFalse(values.hasNext());
+ if((getIterationBehaviour() & AbstractCollectionTest.UNORDERED) == 0){
Review Comment:
@aherbert Got it. Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] aherbert commented on a diff in pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #353:
URL: https://github.com/apache/commons-collections/pull/353#discussion_r1012827480
##########
src/test/java/org/apache/commons/collections4/bidimap/AbstractBidiMapTest.java:
##########
@@ -265,16 +266,18 @@ private void removeValue(final BidiMap<?, ?> map, final Object value) {
@Test
public void testBidiKeySetValuesOrder() {
- resetFull();
- final Iterator<K> keys = map.keySet().iterator();
- final Iterator<V> values = map.values().iterator();
- while (keys.hasNext() && values.hasNext()) {
- final K key = keys.next();
- final V value = values.next();
- assertSame(map.get(key), value);
- }
- assertFalse(keys.hasNext());
- assertFalse(values.hasNext());
+ if((getIterationBehaviour() & AbstractCollectionTest.UNORDERED) == 0){
Review Comment:
Using an if statement does not allow reporting that the test has been skipped. Can you revert and use the previous suggestion to skip based on an assumption (Junit5):
```Java
public void testBidiKeySetValuesOrder() {
// Skip if collection is unordered
Assumptions.assumeFalse((getIterationBehaviour() & AbstractCollectionTest.UNORDERED) != 0);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] anantdahiya8 commented on a diff in pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
anantdahiya8 commented on code in PR #353:
URL: https://github.com/apache/commons-collections/pull/353#discussion_r1007572671
##########
src/main/java/org/apache/commons/collections4/bidimap/DualHashBidiMap.java:
##########
@@ -22,6 +22,7 @@
import java.io.Serializable;
import java.util.HashMap;
import java.util.Map;
+import java.util.LinkedMap;
Review Comment:
Yes, thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] anantdahiya8 commented on pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
anantdahiya8 commented on PR #353:
URL: https://github.com/apache/commons-collections/pull/353#issuecomment-1294402464
> CI build is broken due to wrong import.
Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] anantdahiya8 commented on pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
anantdahiya8 commented on PR #353:
URL: https://github.com/apache/commons-collections/pull/353#issuecomment-1304417036
@aherbert Thanks. Updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-collections] aherbert merged pull request #353: [COLLECTIONS-836] fix flaky test DualHashBidiMapTest
Posted by GitBox <gi...@apache.org>.
aherbert merged PR #353:
URL: https://github.com/apache/commons-collections/pull/353
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org