Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/1045#discussion_r162246947
--- Diff: exec/java-exec/src/test/java/org/apache/drill/PlanningBase.java
---
@@ -84,28 +82,17 @@ protected void testSqlPlan(String sqlCommands) throws
Exception {
systemOptions.init();
@SuppressWarnings("resource")
final UserSession userSession =
UserSession.Builder.newBuilder().withOptionManager(systemOptions).build();
- final SessionOptionManager sessionOptions = (SessionOptionManager)
userSession.getOptions();
+ final SessionOptionManager sessionOptions = userSession.getOptions();
final QueryOptionManager queryOptions = new
QueryOptionManager(sessionOptions);
final ExecutionControls executionControls = new
ExecutionControls(queryOptions, DrillbitEndpoint.getDefaultInstance());
- new NonStrictExpectations() {
- {
- dbContext.getMetrics();
- result = new MetricRegistry();
- dbContext.getAllocator();
- result = allocator;
- dbContext.getConfig();
- result = config;
- dbContext.getOptionManager();
- result = systemOptions;
- dbContext.getStoreProvider();
- result = provider;
- dbContext.getClasspathScan();
- result = scanResult;
- dbContext.getLpPersistence();
- result = logicalPlanPersistence;
- }
- };
+ when(dbContext.getMetrics()).thenReturn(new MetricRegistry());
+ when(dbContext.getAllocator()).thenReturn(allocator);
+ when(dbContext.getConfig()).thenReturn(config);
+ when(dbContext.getOptionManager()).thenReturn(systemOptions);
+ when(dbContext.getStoreProvider()).thenReturn(provider);
+ when(dbContext.getClasspathScan()).thenReturn(scanResult);
+ when(dbContext.getLpPersistence()).thenReturn(logicalPlanPersistence);
--- End diff --
Here, we should have a test time drillbit context implementation that
extends a common interface -- just as we are now doing for the fragment context.
Why? We want classes to have well-defined interfaces, and want to minimize
cohesion. (Basic OO design rules.) It should be made very clear when some
planning test needs additional info. And, if it needs that info, then the set
of info should be varied to ensure all code paths are tested. That can't be
done if we use one generic set of global state for all tests.
This gets to philosophy also. Our tests should strive to cover all possible
states. that Can only be done at the unit level. And, it can only be done if we
drive the tests with a wide variety of inputs.
I we bolkt things together, then tests, we must run a minimum of tests:
just enough to show that "things work" for one or two cases. We then rely on
the users for more complex test cases. That is great -- but tends to upset the
users...
---
