Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/923#discussion_r136662766
--- Diff:
exec/java-exec/src/test/java/org/apache/drill/test/OperatorFixture.java ---
@@ -90,13 +93,13 @@
public static class OperatorFixtureBuilder
{
ConfigBuilder configBuilder = new ConfigBuilder();
- TestOptionSet options = new TestOptionSet();
+ TestOptionManager options = new TestOptionManager();
--- End diff --
This is a bit tricky. Some background.
Most of our existing "unit" (really, system) tests start a Drillbit in
order to test anything.
We are trying (slowly!) to move to true unit tests. To do so, we need to
introduce lose coupling between modules. The full option managers have
dependencies on the Drillbit which sucks in all of Drill.
The goal of the `OptionSet` is to provide a read-only view of the options
since accessing options is pretty simple; it does not have dependencies on the
Drillbit.
We need to preserve this view to allow unit testing. We can do that either
by ensuring that the option managers are stand-alone (have no runtime
dependencies on the Drillbit or the DrillbitContext), or by continuing to do
what this code does: create a test-time, read-only mock version that implements
the option reader (`OptionSet`) interface.
Either are valid approaches; which do you think would work best?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
