apache / accumulo

Apache Accumulo
https://accumulo.apache.org
Apache License 2.0
1.07k stars 445 forks source link

Refactor MultipleStoresIT #4903

Closed kevinrr888 closed 3 weeks ago

kevinrr888 commented 2 months ago

Is your feature request related to a problem? Please describe. MultipleStoresIT should be refactored to function more similarly to how other Fate tests are run (e.g., FateIT) where the two stores are tested in their own separate concrete classes. The existing tests use FateTestRunner, but that doesn't work as nicely here since those function under the assumption of one of each store type, which is not the case with this new test.

Describe the solution you'd like MultipleStoresIT to be an abstract class implemented by two new classes: UserMultipleStoresIT and MetaMultipleStoresIT, and a new interface similar to how FateTestRunner is currently used.

Describe alternatives you've considered The current impl is another alternative, which works but isn't as nice as the above suggestion: it isn't consistent with the structure of the existing Fate tests, and consider this in the current impl:

@Test
public void testReserveUnreserve() throws Exception {
    testReserveUnreserve(FateInstanceType.META);
    testReserveUnreserve(FateInstanceType.USER);
}

If the first call fails, USER will not be run. Would be nicer to have both run irrespective of order and failures.

Additional context See comment https://github.com/apache/accumulo/pull/4524#discussion_r1702305419 See changes from https://github.com/apache/accumulo/pull/4524