[jira] [Commented] (COLLECTIONS-802) ReferenceMap iterator remove violates contract
[
https://issues.apache.org/jira/browse/COLLECTIONS-802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17527256#comment-17527256
]
Bruno P. Kinoshita commented on COLLECTIONS-802:
ApacheMapTest failed for me. Left a comment in the pull request with the diff
applied to pom.xml to get Guava's testlib into the code, and also added Apache
License header & replaced var statements to run everything with `mvn clean
test`. Thanks for the comment above [~ben.manes] ! [~samabcde] thanks for the
PR. Take a look at the comment and let's see if we can fix this issue without
any errors in ApacheMapTest :)
Bruno
> ReferenceMap iterator remove violates contract
> --
>
> Key: COLLECTIONS-802
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
> Project: Commons Collections
> Issue Type: Bug
> Components: Map
>Affects Versions: 4.4
>Reporter: Ben Manes
>Priority: Minor
> Attachments: ApacheMapTest.java
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types.
> This uncovered a contract bug where {{Iterator.remove()}} is invalidated by
> {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming
> {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
> var map = new ReferenceMap<>();
> map.put(1, 2);
> var iter = map.entrySet().iterator();
> assertTrue(iter.hasNext());
> assertTrue(iter.hasNext());
> assertEquals(iter.next(), 1);
> assertFalse(iter.hasNext());
> iter.remove();
> assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib]
> has good coverage for the Collections Framework and might be worth
> integrating. The simple test case that I wrote is attached.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Commented] (COLLECTIONS-802) ReferenceMap iterator remove violates contract
[
https://issues.apache.org/jira/browse/COLLECTIONS-802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17527209#comment-17527209
]
Ben Manes commented on COLLECTIONS-802:
---
Thanks! Can you run {{ApacheMapTest}} locally then to make sure that your
changes didn't break something else?
> ReferenceMap iterator remove violates contract
> --
>
> Key: COLLECTIONS-802
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
> Project: Commons Collections
> Issue Type: Bug
> Components: Map
>Affects Versions: 4.4
>Reporter: Ben Manes
>Priority: Minor
> Attachments: ApacheMapTest.java
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types.
> This uncovered a contract bug where {{Iterator.remove()}} is invalidated by
> {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming
> {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
> var map = new ReferenceMap<>();
> map.put(1, 2);
> var iter = map.entrySet().iterator();
> assertTrue(iter.hasNext());
> assertTrue(iter.hasNext());
> assertEquals(iter.next(), 1);
> assertFalse(iter.hasNext());
> iter.remove();
> assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib]
> has good coverage for the Collections Framework and might be worth
> integrating. The simple test case that I wrote is attached.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Commented] (COLLECTIONS-802) ReferenceMap iterator remove violates contract
[
https://issues.apache.org/jira/browse/COLLECTIONS-802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17527207#comment-17527207
]
Ng Tsz Sum commented on COLLECTIONS-802:
I raised a PR https://github.com/apache/commons-collections/pull/300 to fix the
problem.
I didn't add attached ApacheMapTest as I'm not sure if we can include
*com.google.common.collect.testing* dependency in this project.
> ReferenceMap iterator remove violates contract
> --
>
> Key: COLLECTIONS-802
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
> Project: Commons Collections
> Issue Type: Bug
> Components: Map
>Affects Versions: 4.4
>Reporter: Ben Manes
>Priority: Minor
> Attachments: ApacheMapTest.java
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types.
> This uncovered a contract bug where {{Iterator.remove()}} is invalidated by
> {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming
> {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
> var map = new ReferenceMap<>();
> map.put(1, 2);
> var iter = map.entrySet().iterator();
> assertTrue(iter.hasNext());
> assertTrue(iter.hasNext());
> assertEquals(iter.next(), 1);
> assertFalse(iter.hasNext());
> iter.remove();
> assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib]
> has good coverage for the Collections Framework and might be worth
> integrating. The simple test case that I wrote is attached.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Commented] (COLLECTIONS-802) ReferenceMap iterator remove violates contract
[
https://issues.apache.org/jira/browse/COLLECTIONS-802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17526562#comment-17526562
]
Ben Manes commented on COLLECTIONS-802:
---
Caffeine's
[implementation|https://github.com/ben-manes/caffeine/blob/f52062955adb749e7828d1f1cff12a9eea26bee4/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java#L3452-L3455]
also modified state during the hasNext(), but it caches the next entry and
uses that as a guard condition to only search when null.
> ReferenceMap iterator remove violates contract
> --
>
> Key: COLLECTIONS-802
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
> Project: Commons Collections
> Issue Type: Bug
> Components: Map
>Affects Versions: 4.4
>Reporter: Ben Manes
>Priority: Minor
> Attachments: ApacheMapTest.java
>
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types.
> This uncovered a contract bug where {{Iterator.remove()}} is invalidated by
> {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming
> {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
> var map = new ReferenceMap<>();
> map.put(1, 2);
> var iter = map.entrySet().iterator();
> assertTrue(iter.hasNext());
> assertTrue(iter.hasNext());
> assertEquals(iter.next(), 1);
> assertFalse(iter.hasNext());
> iter.remove();
> assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib]
> has good coverage for the Collections Framework and might be worth
> integrating. The simple test case that I wrote is attached.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Commented] (COLLECTIONS-802) ReferenceMap iterator remove violates contract
[
https://issues.apache.org/jira/browse/COLLECTIONS-802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17526546#comment-17526546
]
Ben Manes commented on COLLECTIONS-802:
---
To clarify, the attached ApacheMapTest is a JUnit-3 test case that covers the
entire collections interface. You might consider incorporating it into your
test suite.
> ReferenceMap iterator remove violates contract
> --
>
> Key: COLLECTIONS-802
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
> Project: Commons Collections
> Issue Type: Bug
> Components: Map
>Affects Versions: 4.4
>Reporter: Ben Manes
>Priority: Minor
> Attachments: ApacheMapTest.java
>
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types.
> This uncovered a contract bug where {{Iterator.remove()}} is invalidated by
> {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming
> {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
> var map = new ReferenceMap<>();
> map.put(1, 2);
> var iter = map.entrySet().iterator();
> assertTrue(iter.hasNext());
> assertTrue(iter.hasNext());
> assertEquals(iter.next(), 1);
> assertFalse(iter.hasNext());
> iter.remove();
> assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib]
> has good coverage for the Collections Framework and might be worth
> integrating. The simple test case that I wrote is attached.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
[jira] [Commented] (COLLECTIONS-802) ReferenceMap iterator remove violates contract
[
https://issues.apache.org/jira/browse/COLLECTIONS-802?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17526507#comment-17526507
]
Ng Tsz Sum commented on COLLECTIONS-802:
runnable with java 8, same result as stated
{code:java}
@Test
public void iterator_remove_failed_after_hasNext() {
ReferenceMap map = new ReferenceMap<>();
map.put(1, 2);
Iterator> iter = map.entrySet().iterator();
assertTrue(iter.hasNext());
assertTrue(iter.hasNext());
assertEquals(iter.next().getValue(), new Integer(2));
// comment below hasNext() call will pass
assertFalse(iter.hasNext());
iter.remove();
assertTrue("Expect empty but have entry: " + map.toString(),
map.isEmpty());
}
{code}
> ReferenceMap iterator remove violates contract
> --
>
> Key: COLLECTIONS-802
> URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
> Project: Commons Collections
> Issue Type: Bug
> Components: Map
>Affects Versions: 4.4
>Reporter: Ben Manes
>Priority: Minor
> Attachments: ApacheMapTest.java
>
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types.
> This uncovered a contract bug where {{Iterator.remove()}} is invalidated by
> {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming
> {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
> var map = new ReferenceMap<>();
> map.put(1, 2);
> var iter = map.entrySet().iterator();
> assertTrue(iter.hasNext());
> assertTrue(iter.hasNext());
> assertEquals(iter.next(), 1);
> assertFalse(iter.hasNext());
> iter.remove();
> assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib]
> has good coverage for the Collections Framework and might be worth
> integrating. The simple test case that I wrote is attached.
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
