drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #923: DRILL-5723: Added System Internal Options That can ...
Date Fri, 01 Sep 2017 20:54:53 GMT
Github user paul-rogers commented on a diff in the pull request:

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

View raw message