Parameterise DataLoaderTest on DataLoader #153
Conversation
The existing `DataLoaderTest` covers a very large range of test cases which will be very useful to validate what is being added in graphql-java#148. To allow new `*Publisher` `DataLoader`s to leverage this without copy-pasting, we make `DataLoaderTest` a parameterised test, using two `DataLoader` variants: - the stock 'List' `DataLoader`. - the 'Mapped' `DataLoader`. Most of the tests in `DataLoaderTest` have been retrofitted for this parameterisation, resulting in: - deduplicated code (many of the `MappedDataLoader` tests have the same test cases, almost line-for-line). - increased coverage of the `MappedDataLoader`, bolstering confidence in subsequent changes (rather than relying on the author understanding how everything is put together internally).
|
|
||
| private static <K, V> DataLoader<K, V> idMapLoaderBlowsUps( | ||
| DataLoaderOptions options, List<Collection<K>> loadCalls) { | ||
| return newDataLoader((keys) -> { |
There was a problem hiding this comment.
I'm not sure this was testing a MappedBatchLoader - we would have needed newMappedDataLoader for this.
|
|
||
|
|
||
| @Test | ||
| public void basic_map_batch_loading() { |
There was a problem hiding this comment.
I've moved this to DataLoaderTest - all other test cases here were identical to the ones found in DataLoaderTest.
| @@ -78,9 +88,36 @@ public void should_Build_a_really_really_simple_data_loader() { | |||
| } | |||
|
|
|||
| @Test | |||
| public void should_Support_loading_multiple_keys_in_one_call() { | |||
| public void basic_map_batch_loading() { | |||
There was a problem hiding this comment.
Moved across from DataLoaderMapBatchLoaderTest.
| } | ||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("dataLoaderFactories") |
There was a problem hiding this comment.
We now run this test (and any test annotated with @MethodSource("dataLoaderFactories")) with all the TestDataLoaderFactory instances returned from the static method dataLoaderFactories.
| private static Stream<Arguments> dataLoaderFactories() { | ||
| return Stream.of( | ||
| Arguments.of(Named.of("List DataLoader", new ListDataLoaderFactory())), | ||
| Arguments.of(Named.of("Mapped DataLoader", new MappedDataLoaderFactory())) |
There was a problem hiding this comment.
Now, if we wish to test more implementations of a DataLoader, we need only drop in a new entry here, and we get instant coverage.
| return TestKit.futureError(); | ||
| }, options); | ||
| public interface TestDataLoaderFactory { | ||
| <K, V> DataLoader<K, V> idLoader(DataLoaderOptions options, List<Collection<K>> loadCalls); |
There was a problem hiding this comment.
These methods are identical to what was already present for the List DataLoader; I've moved across/added new ones for the Mapped implementation.
| AtomicBoolean success = new AtomicBoolean(); | ||
| DataLoader<Integer, Integer> identityLoader = newDataLoader(keysAsValues()); |
There was a problem hiding this comment.
I've swapped this out for idLoader (and for the next test) purely since I think this valuable enough to test for each batch function.
| @@ -665,14 +727,19 @@ public void should_work_with_duplicate_keys_when_caching_disabled() throws Execu | |||
| assertThat(future1.get(), equalTo("A")); | |||
| assertThat(future2.get(), equalTo("B")); | |||
| assertThat(future3.get(), equalTo("A")); | |||
| assertThat(loadCalls, equalTo(singletonList(asList("A", "B", "A")))); | |||
| if (factory instanceof ListDataLoaderFactory) { | |||
There was a problem hiding this comment.
I was a bit iffy on this - wasn't sure if I should split this out into a separate test but it's 95% the same code.
The existing
DataLoaderTestcovers a very large range of test cases which will be very useful to validate what is being added in #148.To allow new
*PublisherDataLoaders to leverage this without copy-pasting, we makeDataLoaderTesta parameterised test, using twoDataLoadervariants:DataLoader.DataLoader.Most of the tests in
DataLoaderTesthave been retrofitted for this parameterisation, resulting in:MappedDataLoadertests have the same test cases, almost line-for-line).MappedDataLoader, bolstering confidence in subsequent changes (rather than relying on the author understanding how everything is put together internally).