Introduce component test suite (#108)

* Document test suite differences

* Refine definition of component test

* Drop unneeded patch

* Move tests to component suite

* Split up test file between suites

* Add __init__.py in new directories

* Move test to component suite

* Fix missing resources

* Add missing __init__.py files

* Combine coverage

Co-authored-by: Vince Broz <[email protected]>

Makefile

@@ -24,11 +24,31 @@ citypecoverage: typecoverage
 	@git status --porcelain metrics/mypy_high_water_mark
 	@test -z "$$(git status --porcelain metrics/mypy_high_water_mark)"
 
-test:
-	ENV=test nosetests --cover-package=records_mover --with-coverage --with-xunit --cover-html --cover-xml --cover-inclusive tests/unit
-
-citest: test-reports
-	ENV=test nosetests --cover-package=records_mover --with-coverage --with-xunit --cover-html --cover-xml --cover-inclusive --xunit-file=test-reports/junit.xml tests/unit
+unit:
+	ENV=test nosetests --cover-package=records_mover --cover-erase --with-coverage --with-xunit --cover-html --cover-xml --cover-inclusive tests/unit
+	mv .coverage .coverage-unit
+
+component:
+	ENV=test nosetests --cover-package=records_mover --with-coverage --with-xunit --cover-html --cover-xml --cover-inclusive tests/component
+	mv .coverage .coverage-component
+
+test: unit component
+	coverage combine .coverage-unit .coverage-component # https://stackoverflow.com/questions/7352319/nosetests-combined-coverage
+	coverage html --directory=cover
+	coverage xml
+
+ciunit:
+	ENV=test nosetests --cover-package=records_mover --cover-erase --with-coverage --with-xunit --cover-html --cover-xml --cover-inclusive --xunit-file=test-reports/junit.xml tests/unit
+	mv .coverage .coverage-unit
+
+cicomponent:
+	ENV=test nosetests --cover-package=records_mover --with-coverage --with-xunit --cover-html --cover-xml --cover-inclusive --xunit-file=test-reports/junit.xml tests/component
+	mv .coverage .coverage-component
+
+citest: test-reports ciunit cicomponent
+	coverage combine .coverage-unit .coverage-component # https://stackoverflow.com/questions/7352319/nosetests-combined-coverage
+	coverage html --directory=cover
+	coverage xml
 
 coverage:
 	python setup.py coverage_ratchet

Rakefile.quality

@@ -31,5 +31,5 @@ Quality::Rake::Task.new do |t|
   ]
 
   t.source_files_exclude_glob =
-    '{cover/**/*,tests/integration/records/records_schema_v1_schema.json,tests/unit/records/schema/future_incompatible_redshift_example_1.json,tests/unit/records/schema/pandas_example_1.json,tests/unit/records/schema/redshift_example_1.json,records_mover/db/bigquery/load_job_config_options.py,records_mover/records/pandas/read_csv_options.py,.circleci/config.yml,CODE_OF_CONDUCT.md,LICENSE,types/stubs/inspect.pyi,records_mover/db/postgres/sqlalchemy_postgres_copy.py,docs/schema/redshift_example_1.json,docs/schema/pandas_example_1.json,docs/RECORDS_SPEC.md,docs/schema/SCHEMAv1.md,docs/records-mover-horizontal.png}'
+    '{cover/**/*,tests/integration/records/records_schema_v1_schema.json,tests/component/records/schema/future_incompatible_redshift_example_1.json,tests/component/records/schema/pandas_example_1.json,tests/component/records/schema/redshift_example_1.json,records_mover/db/bigquery/load_job_config_options.py,records_mover/records/pandas/read_csv_options.py,.circleci/config.yml,CODE_OF_CONDUCT.md,LICENSE,types/stubs/inspect.pyi,records_mover/db/postgres/sqlalchemy_postgres_copy.py,docs/schema/redshift_example_1.json,docs/schema/pandas_example_1.json,docs/RECORDS_SPEC.md,docs/schema/SCHEMAv1.md,docs/records-mover-horizontal.png}'
 end

metrics/coverage_high_water_mark

@@ -1 +1 @@
-93.7200
+93.4700

tests/README.md

@@ -0,0 +1,72 @@
+# Tests
+
+## Test suites
+
+* unit: Tests which interact with the methods of a single
+  class/module, and use mocks to assert that side-effects occur -
+  either on arguments passed in, or on dependencies of the
+  class/module.  These tests mock anything underneath that touches the
+  filesystem or database, and any non-trivial dependencies of the
+  class.  See
+  [Sandi Metz' advice for unit testing](https://www.youtube.com/watch?v=URSWYvyc42M)
+  for the philosphy used in this suite.
+
+* component: Tests which minimize their use of mocks to the following
+  situations:
+
+  * Mocks representing 'this argument/dependency won't be used'.
+  * Mocks used as unique values to verify inputs get mapped to certain outputs.
+  * Mocks used to avoid I/O - e.g., touching the filesystem, network or database
+
+  These tests may interact with multiple classes/modules, including
+  those classes whose public methods may not be considered part of the
+  Records Mover public API.
+
+  The idea with this suite is that it can be used to test at more
+  natural levels of interaction than at the class/module level.
+
+  Since much of the integration test suite is per-database, this is
+  also a good place to test larger system behavior that isn't
+  database-specific to minimize test time/cost.
+
+  Note that tests in the 'unit' suite make small scale (within the
+  same class) refactors very safe, they are fragile when refactors
+  among different classes are made.
+
+  If you are performing a refactor among different classes and want
+  automated test support, adding a component test at a higher level to
+  the changed classes is a good option.  At that point, you can safely
+  remove the failing unit tests.
+
+  If adding a component test isn't an easy option, it may be worth a
+  little time to see if there's a natural higher level interface that
+  can be added that would change that situation (not guanteed, but
+  worth a look!).
+
+* integration: Tests which interact with the API at the highest level,
+  or use the CLI.  These tests can touch databases, the network, etc,
+  in controlled ways (e..g, using Docker or specific test accounts in
+  cloud databases).
+
+  Since Records Mover testing aims to ensure consistent behavior
+  against different databases with different formats of data, this is
+  an important suite than can reveal complex interaction issues.
+  Records Mover doesn't comply with a traditional test pyramid in that
+  sense, but uses extensive parallelism in CircleCI to minimize test
+  time.  Despite the provocative title,
+  [Unit testing is overrated](https://tyrrrz.me/blog/unit-testing-is-overrated)
+  describes the trade-offs and rationale to this approach.
+
+  That said, adding new tests to this suite should be done sparingly,
+  especially as many of the tests make sense to perform against
+  multiple database types (`n`), data input types (`d`), and
+  sources/targets (`s`/`t`) - right now our suite is `O(n^2)` for the
+  database-to-database tests, `O(d*n)` for the database import tests,
+  and `O(s*t)` for the CLI "any2any" tests.
+
+  All of these values are expected to rise in the future, so this all
+  adds up quick, even in the cloud!
+
+  Consider adding tests first at the unit level for detailed class
+  behavior, or ideally at the component level if the behavior can
+  exercised with minimal use of mocking.

tests/component/records/format_hints.py

@@ -0,0 +1,111 @@
+bluelabs_format_hints = {
+    'field-delimiter': ',',
+    'record-terminator': "\n",
+    'compression': 'GZIP',
+    'quoting': None,
+    'quotechar': '"',
+    'doublequote': False,
+    'escape': '\\',
+    'encoding': 'UTF8',
+    'dateformat': 'YYYY-MM-DD',
+    'timeonlyformat': 'HH24:MI:SS',
+    'datetimeformattz': 'YYYY-MM-DD HH:MI:SSOF',
+    'datetimeformat': 'YYYY-MM-DD HH24:MI:SS',
+    'header-row': False,
+}
+
+csv_format_hints = {
+    'field-delimiter': ',',
+    'record-terminator': "\n",
+    'compression': 'GZIP',
+    'quoting': 'minimal',
+    'quotechar': '"',
+    'doublequote': True,
+    'escape': None,
+    'encoding': 'UTF8',
+    'dateformat': 'MM/DD/YY',
+    'timeonlyformat': 'HH24:MI:SS',
+    'datetimeformattz': 'MM/DD/YY HH24:MI',
+    'datetimeformat': 'MM/DD/YY HH24:MI',
+    'header-row': True,
+}
+
+vertica_format_hints = {
+    'field-delimiter': '\001',
+    'record-terminator': '\002',
+    'compression': None,
+    'quoting': None,
+    'quotechar': '"',
+    'doublequote': False,
+    'escape': None,
+    'encoding': 'UTF8',
+    'dateformat': 'YYYY-MM-DD',
+    'timeonlyformat': 'HH24:MI:SS',
+    'datetimeformat': 'YYYY-MM-DD HH:MI:SS',
+    'datetimeformattz': 'YYYY-MM-DD HH:MI:SSOF',
+    'header-row': False,
+}
+
+christmas_tree_format_1_hints = {
+    'field-delimiter': '\001',
+    'record-terminator': '\002',
+    'compression': 'LZO',
+    'quoting': 'nonnumeric',
+    'quotechar': '"',
+    'doublequote': False,
+    'escape': '\\',
+    'encoding': 'UTF8',
+    'dateformat': 'YYYY-MM-DD',
+    'timeonlyformat': 'HH24:MI:SS',
+    'datetimeformat': 'YYYY-MM-DD HH24:MI:SS',
+    'datetimeformattz': 'YYYY-MM-DD HH:MI:SSOF',
+    'header-row': True,
+}
+
+christmas_tree_format_2_hints = {
+    'field-delimiter': '\001',
+    'record-terminator': '\002',
+    'compression': 'BZIP',
+    'quoting': 'all',
+    'quotechar': '"',
+    'doublequote': True,
+    'escape': '@',  # not really allowed in the spec, but let's see what happens
+    'encoding': 'UTF8',
+    'dateformat': 'MM-DD-YYYY',
+    'timeonlyformat': 'HH24:MI:SS',
+    'datetimeformat': 'YYYY-MM-DD HH24:MI:SS',
+    'datetimeformattz': 'HH:MI:SSOF YYYY-MM-DD',  # also not allowed
+    'header-row': False,
+}
+
+christmas_tree_format_3_hints = {
+    'field-delimiter': '\001',
+    'record-terminator': '\002',
+    'compression': 'BZIP',
+    'quoting': 'some_future_option_not_supported_now',
+    'quotechar': '"',
+    'doublequote': True,
+    'escape': '@',  # not really allowed in the spec, but let's see what happens
+    'encoding': 'UTF8',
+    'dateformat': 'DD-MM-YYYY',
+    'timeonlyformat': 'HH24:MI:SS',
+    'datetimeformat': 'YYYY-MM-DD HH24:MI:SS',
+    'datetimeformattz': 'HH:MI:SSOF YYYY-MM-DD',  # also not allowed
+    'header-row': False,
+}
+
+christmas_tree_format_4_hints = {
+    'field-delimiter': '\001',
+    'record-terminator': '\002',
+    'compression': 'BZIP',
+    'quoting': 'some_future_option_not_supported_now',
+    'quotechar': '"',
+    'doublequote': True,
+    'escape': '@',  # not really allowed in the spec, but let's see what happens
+    'encoding': 'UTF8',
+    'dateformat': 'totally_bogus_just_made_this_up',
+    'timeonlyformat': 'HH24:MI:SS',
+    'datetimeformat': 'YYYY-MM-DD HH24:MI:SS',
+    'datetimeformattz': 'HH:MI:SSOF YYYY-MM-DD',  # also not allowed
+    'header-row': False,
+}

tests/unit/records/schema/field/test_numpy.py → tests/component/records/schema/field/test_numpy.py

@@ -1,13 +1,11 @@
 import unittest
-from mock import Mock, patch
+from mock import Mock
 from records_mover.records.schema.field.numpy import details_from_numpy_dtype
 import numpy as np
 
 
 class TestNumpy(unittest.TestCase):
-    @patch('records_mover.records.schema.field.numpy.RecordsSchemaFieldConstraints')
-    def test_details_from_numpy_dtype(self,
-                                      mock_RecordsSchemaFieldConstraints):
+    def test_details_from_numpy_dtype(self):
         tests = {
             np.dtype(str): 'string',
             np.dtype(int): 'integer',