[TEST] Adds tests for ESTestCase randomSubset methods#131745
[TEST] Adds tests for ESTestCase randomSubset methods#131745joshua-adams-1 merged 8 commits intoelastic:mainfrom
Conversation
Adds tests for the following methods in `ESTestCase`: `randomSubsetOf`, `randomNonEmptySubsetOf` and `shuffledList`
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
DaveCTurner
left a comment
There was a problem hiding this comment.
I'd rather use more randomness here rather than hard-coding just these specific values.
| List<Integer> result = ESTestCase.randomSubsetOf(2, 1, 2, 3, 4); | ||
| assertEquals(2, result.size()); | ||
| assertTrue(Arrays.asList(1, 2, 3, 4).containsAll(result)); |
There was a problem hiding this comment.
I think we can use randomness to make some more general statements here. This should hold for all values of 2 (the subset size), so why not choose it randomly? Likewise the source list could be of any length, it doesn't have to be {1,2,3,4}.
| List<Integer> result = ESTestCase.randomSubsetOf(0, 1, 2, 3); | ||
| assertEquals(0, result.size()); |
There was a problem hiding this comment.
This should hold for all values of
2
In particular, if we pick 2 randomly from a set including 0 then we'd cover this case at the same time.
| } | ||
|
|
||
| public void testRandomSubsetOfWithVarargsAndSizeTooLarge() { | ||
| assertThrows(IllegalArgumentException.class, () -> ESTestCase.randomSubsetOf(5, 1, 2, 3)); |
There was a problem hiding this comment.
Likewise this is true for all values of 5 greater than 3.
fa5d4a4 to
2044927
Compare
954764d to
abc4581
Compare
| } | ||
|
|
||
| public void testRandomSubsetOfWithVarargs() { | ||
| List<Integer> randomList = randomList(10, () -> randomIntBetween(-100, 100)); |
There was a problem hiding this comment.
Nit: We could also randomize the list length of 10. The same goes for other places where we construct the initial list.
There was a problem hiding this comment.
The 10 parameter is actually the maximum list size, not the hardcoded list size. The randomList function randomises the length between 0 and this value inclusively. I could have randomised this size too, but I figured hardcoding a max list size was acceptable, because even if I randomised it, I would still have to declare a max value somewhere.
…-tracking * upstream/main: Fix MergeWithLowDiskSpaceIT testRelocationWhileForceMerging (elastic#131806) [ML] Prevent the trained model deployment memory estimation from double-counting allocations. (elastic#131990) ES|QL Assert current thread during query planning and execution (elastic#131807) Add ElasticsearchIndexDeletionPolicy and EngineConfig policy wrapper (elastic#130442) [TEST] Adds tests for ESTestCase randomSubset methods (elastic#131745) Simplify esql session (elastic#131925) Simplify EsqlExecution info serialization (elastic#131823) Add utility to check for project global block (elastic#131927) [DOCS] Update ES|QL applies to's (elastic#131805) Handle structured log messages (elastic#131027) Mute org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT test {p0=search/600_flattened_ignore_above/flattened ignore_above multi-value field} elastic#131967 Mute org.elasticsearch.xpack.remotecluster.CrossClusterEsqlRCS2EnrichUnavailableRemotesIT testEsqlEnrichWithSkipUnavailable elastic#131965 Mute org.elasticsearch.xpack.restart.FullClusterRestartIT testWatcherWithApiKey {cluster=UPGRADED} elastic#131964 [ES|QL] Fix aggregate_metric_double sorting and mv_expand issues (elastic#131658) Reduce logging levels for meter usage tests (elastic#131935)
[TEST] Adds tests for ESTestCase randomSubset methods Adds tests for the following methods in `ESTestCase`: `randomSubsetOf`, `randomNonEmptySubsetOf` and `shuffledList`
Adds tests for the following methods in
ESTestCase:randomSubsetOf,randomNonEmptySubsetOfandshuffledListRelates #126579