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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---