testlib: Add concurrent collection spliterator tests via derived suite builders#8173
Conversation
| Spliterator<?> spliterator = getMap().values().spliterator(); | ||
| assertTrue( | ||
| "values().spliterator() should have CONCURRENT characteristic", | ||
| spliterator.hasCharacteristics(Spliterator.CONCURRENT)); |
There was a problem hiding this comment.
I think we can move the testEntrySetSpliteratorNotConcurrentAndSized, testKeySetSpliteratorNotConcurrentAndSized, and testValuesSpliteratorNotConcurrentAndSized tests to CollectionSpliteratorTester instead.
Per https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/Spliterator.html#CONCURRENT,
A top-level Spliterator should not report both
CONCURRENTandSIZED
A top-level Spliterator should not report bothCONCURRENTandIMMUTABLE
and that should apply to all top-level Spliterators, not just concurrent ones.
Also, https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/Spliterator.html#SUBSIZED says
A Spliterator that does not report
SIZEDas required bySUBSIZEDis inconsistent
We can check that as well.
| * Tests that if the spliterator reports {@code SORTED}, it also reports {@code ORDERED}, as | ||
| * required by the {@link Spliterator} specification. | ||
| */ | ||
| public void testSpliteratorSortedRequiresOrdered() { |
There was a problem hiding this comment.
Good catch! Thanks for adding this as well.
| ConcurrentMapRemoveTester.class, | ||
| ConcurrentMapReplaceTester.class, | ||
| ConcurrentMapReplaceEntryTester.class); | ||
| ConcurrentMapReplaceEntryTester.class, |
There was a problem hiding this comment.
Instead of adding a new tester here, we can maybe override the
createDerivedEntrySetSuitecreateDerivedKeySetSuitecreateDerivedValueCollectionSuite
methods to append a ConcurrentCollectionSpliteratorTester, so it could be reused for arbitrary concurrent collections.
…istics Add tests to verify that ConcurrentMap implementations return spliterators with the CONCURRENT characteristic on their entrySet(), keySet(), and values() views. Also verifies that no spliterator reports both CONCURRENT and SIZED characteristics (which are mutually incompatible per the Java specification). Fixes google#7855
Per review feedback — the spec invariants apply to all spliterators, not just ConcurrentMap ones, so they belong in the common tester. Added checks for all four spec-mandated invariants: - CONCURRENT + SIZED must not coexist - CONCURRENT + IMMUTABLE must not coexist - SUBSIZED requires SIZED - SORTED requires ORDERED Stripped ConcurrentMapSpliteratorTester down to just the CONCURRENT-must-exist assertions and dropped the reflection methods to match the other ConcurrentMap*Tester classes.
Instead of adding ConcurrentMapSpliteratorTester to the map tester list, use the derived suite architecture as @chaoren suggested: - Add ConcurrentCollectionSpliteratorTester that tests any collection's spliterator for the CONCURRENT characteristic - Add ConcurrentSetTestSuiteBuilder and ConcurrentCollectionTestSuiteBuilder that include this tester - Override createDerivedEntrySetSuite/KeySetSuite/ValueCollectionSuite in ConcurrentMapTestSuiteBuilder to use the concurrent builders - Delete ConcurrentMapSpliteratorTester (no longer needed) This approach is more composable - the tester can be reused for any concurrent collection, not just ConcurrentMap views.
nickita-khylkouski
force-pushed
the
add-concurrent-map-spliterator-tests
branch
from
fcdfd31 to
9c3cb22
Compare
nickita-khylkouski
changed the title
|
Done - moved the spec invariant checks to CollectionSpliteratorTester and set up the derived suite overrides so the concurrent builders get used for entrySet/keySet/values. Rebased on latest master too. Let me know if anything else needs adjusting. |
3 participants
Summary
Adds testlib support for verifying
Spliteratorcharacteristics onConcurrentMapviews.Architecture (per @chaoren's feedback):
ConcurrentCollectionSpliteratorTester- tests that a collection's spliterator has theCONCURRENTcharacteristicConcurrentSetTestSuiteBuilderandConcurrentCollectionTestSuiteBuilder- reusable builders that include the concurrent testercreateDerivedEntrySetSuite,createDerivedKeySetSuite,createDerivedValueCollectionSuiteinConcurrentMapTestSuiteBuilderto use the concurrent buildersCollectionSpliteratorTester(applies to ALL collections):CONCURRENTandSIZEDmust not coexistCONCURRENTandIMMUTABLEmust not coexistSUBSIZEDrequiresSIZEDSORTEDrequiresORDEREDMotivation
Per issue #7855 and maintainer guidance from @chaoren:
ConcurrentMapimplementations should return spliterators with theCONCURRENTcharacteristicCurrently, some Guava
ConcurrentMapimplementations (e.g.,LocalCache,MapMakerInternalMap) return spliterators that:CONCURRENT(should have it)SIZED(incompatible withCONCURRENTper spec)This PR adds test infrastructure to document the expected behavior. A follow-up PR can fix the implementations.
Testing
ConcurrentMap*Testerpattern (plain@GwtCompatible,@NullMarked, no reflection methods)ConcurrentHashMapcorrectly returnsCONCURRENT=true, SIZED=falseFixes #7855