move test and fixtures to root level and simplify test-install command (#4264)

* simplify test-install command

* update CHANGELOG

* fix MANIFEST.in

* move tests and fixtures to root level

* fix archive fixture

* update coverage configs

Author: epwalsh

.coveragerc

@@ -1,2 +1,2 @@
 [run]
-omit = allennlp/tests/*
+omit = tests/*

.flake8

@@ -18,7 +18,7 @@ per-file-ignores =
 
     # tests don't have to respect
     #  E731: do not assign a lambda expression, use a def
-    allennlp/tests/**:E731
+    tests/**:E731
 
     # scripts don't have to respect
     #  E402: imports not at top of file (because we mess with sys.path)

CHANGELOG.md

@@ -2,7 +2,6 @@ include LICENSE
 include README.md
 recursive-include allennlp *
 recursive-include scripts *
-recursive-include training_config *.json
-recursive-include tutorials *
-global-exclude .DS_Store
-global-exclude __pycache__
+global-exclude .DS_Store *.py[cod]
+prune **/__pycache__
+prune **/.mypy_cache

MANIFEST.in

@@ -38,11 +38,11 @@ version :
 
 .PHONY : lint
 lint :
-	flake8 ./scripts $(SRC)
+	flake8 ./scripts ./tests $(SRC)
 
 .PHONY : format
 format :
-	black --check ./scripts $(SRC)
+	black --check ./scripts ./tests $(SRC)
 
 .PHONY : typecheck
 typecheck :
@@ -54,18 +54,18 @@ typecheck :
 
 .PHONY : test
 test :
-	pytest --color=yes -rf --durations=40 $(SRC) scripts/
+	pytest --color=yes -rf --durations=40
 
 .PHONY : test-with-cov
 test-with-cov :
 	pytest --color=yes -rf --durations=40 \
 			--cov-config=.coveragerc \
 			--cov=$(SRC) \
-			--cov-report=xml $(SRC) scripts/
+			--cov-report=xml
 
 .PHONY : gpu-test
 gpu-test :
-	pytest --color=yes -v -rf -m gpu $(SRC)
+	pytest --color=yes -v -rf -m gpu
 
 #
 # Setup helpers
@@ -130,6 +130,11 @@ clean :
 	rm -rf $(MD_DOCS_TGT)
 	rm -rf $(MD_DOCS_API_ROOT)
 	rm -f $(MD_DOCS_ROOT)*.md
+	rm -rf .pytest_cache/
+	rm -rf allennlp.egg-info/
+	rm -rf dist/
+	rm -rf build/
+	find . | grep -E '(\.mypy_cache|__pycache__|\.pyc|\.pyo$$)' | xargs rm -rf
 
 #
 # Docker helpers.

Makefile

@@ -1,20 +1,19 @@
 """
-The `test-install` subcommand verifies
-an installation by running the unit tests.
+The `test-install` subcommand provides a programmatic way to verify
+that AllenNLP has been successfully installed.
 """
 
 import argparse
 import logging
-import os
 import pathlib
-import sys
 
-import pytest
 from overrides import overrides
+import torch
 
 import allennlp
+from allennlp.common.util import import_module_and_submodules
 from allennlp.commands.subcommand import Subcommand
-from allennlp.common.util import pushd
+from allennlp.version import VERSION
 
 
 logger = logging.getLogger(__name__)
@@ -24,24 +23,11 @@
 class TestInstall(Subcommand):
     @overrides
     def add_subparser(self, parser: argparse._SubParsersAction) -> argparse.ArgumentParser:
-
-        description = """Test that installation works by running the unit tests."""
+        description = """Test that AllenNLP is installed correctly."""
         subparser = parser.add_parser(
-            self.name, description=description, help="Run the unit tests."
-        )
-
-        subparser.add_argument(
-            "--run-all",
-            action="store_true",
-            help="By default, we skip tests that are slow "
-            "or download large files. This flag will run all tests.",
-        )
-        subparser.add_argument(
-            "-k", type=str, default=None, help="Limit tests by setting pytest -k argument"
+            self.name, description=description, help="Test AllenNLP installation."
         )
-
         subparser.set_defaults(func=_run_test)
-
         return subparser
 
 
@@ -50,23 +36,14 @@ def _get_module_root():
 
 
 def _run_test(args: argparse.Namespace):
-    module_parent = _get_module_root().parent
-    logger.info("Changing directory to %s", module_parent)
-    with pushd(module_parent):
-        test_dir = os.path.join(module_parent, "allennlp")
-        logger.info("Running tests at %s", test_dir)
-
-        if args.k:
-            pytest_k = ["-k", args.k]
-            pytest_m = ["-m", "not java"]
-            if args.run_all:
-                logger.warning("the argument '-k' overwrites '--run-all'.")
-        elif args.run_all:
-            pytest_k = []
-            pytest_m = []
-        else:
-            pytest_k = ["-k", "not sniff_test"]
-            pytest_m = ["-m", "not java"]
-
-        exit_code = pytest.main([test_dir, "--color=no"] + pytest_k + pytest_m)
-        sys.exit(exit_code)
+    # Make sure we can actually import the main modules without errors.
+    import_module_and_submodules("allennlp.common")
+    import_module_and_submodules("allennlp.data")
+    import_module_and_submodules("allennlp.interpret")
+    import_module_and_submodules("allennlp.models")
+    import_module_and_submodules("allennlp.modules")
+    import_module_and_submodules("allennlp.nn")
+    import_module_and_submodules("allennlp.predictors")
+    import_module_and_submodules("allennlp.training")
+    logger.info("AllenNLP version %s installed to %s", VERSION, _get_module_root())
+    logger.info("Cuda devices available: %s", torch.cuda.device_count())

allennlp/commands/test_install.py

@@ -19,8 +19,8 @@ class AllenNlpTestCase:
     PROJECT_ROOT = (pathlib.Path(__file__).parent / ".." / ".." / "..").resolve()
     MODULE_ROOT = PROJECT_ROOT / "allennlp"
     TOOLS_ROOT = MODULE_ROOT / "tools"
-    TESTS_ROOT = MODULE_ROOT / "tests"
-    FIXTURES_ROOT = TESTS_ROOT / "fixtures"
+    TESTS_ROOT = PROJECT_ROOT / "tests"
+    FIXTURES_ROOT = PROJECT_ROOT / "test_fixtures"
 
     def setup_method(self):
         logging.basicConfig(

allennlp/common/testing/test_case.py

@@ -11,4 +11,4 @@ coverage:
     changes: false
 comment: false
 ignore:
-  - "allennlp/tests/"
+  - "tests/"

allennlp/tests/data/dataset_readers/semantic_dependency_parsing.py

@@ -1,5 +1,5 @@
 [pytest]
-testpaths = allennlp/tests/
+testpaths = tests/ scripts/tests/
 python_classes = Test* *Test
 log_format = %(asctime)s - %(levelname)s - %(name)s - %(message)s
 log_level = DEBUG

allennlp/tests/fixtures/basic_classifier/experiment_from_archive.jsonnet

@@ -48,7 +48,9 @@
     author="Allen Institute for Artificial Intelligence",
     author_email="[email protected]",
     license="Apache",
-    packages=find_packages(exclude=["*.tests", "*.tests.*", "tests.*", "tests"]),
+    packages=find_packages(
+        exclude=["*.tests", "*.tests.*", "tests.*", "tests", "test_fixtures", "test_fixtures.*"]
+    ),
     install_requires=[
         "torch>=1.5.0,<1.6.0",
         "jsonnet>=0.10.0 ; sys.platform != 'win32'",

allennlp/tests/fixtures/basic_classifier/from_archive_serialization/model.tar.gz

@@ -14,8 +14,8 @@
         },
         "max_sequence_length": 400
     },
-    "train_data_path": "allennlp/tests/fixtures/data/text_classification_json/imdb_corpus.jsonl",
-    "validation_data_path": "allennlp/tests/fixtures/data/text_classification_json/imdb_corpus.jsonl",
+    "train_data_path": "test_fixtures/data/text_classification_json/imdb_corpus.jsonl",
+    "validation_data_path": "test_fixtures/data/text_classification_json/imdb_corpus.jsonl",
     "data_loader": {
 
         "batch_sampler": {

codecov.yml

@@ -0,0 +1,14 @@
+local COMMON = import 'common.jsonnet';
+
+{
+    "dataset_reader": COMMON['dataset_reader'],
+    "datasets_for_vocab_creation": ["train"],
+    "train_data_path": "test_fixtures/data/text_classification_json/ag_news_corpus_fake_sentiment_labels.jsonl",
+    "validation_data_path": "test_fixtures/data/text_classification_json/ag_news_corpus_fake_sentiment_labels.jsonl",
+    "model": {
+        "type": "from_archive",
+        "archive_file": "test_fixtures/basic_classifier/serialization/model.tar.gz",
+    },
+    "data_loader": COMMON['data_loader'],
+    "trainer": COMMON['trainer'],
+}

pytest.ini

@@ -12,8 +12,8 @@
         }
       }
     },
-    "train_data_path": "allennlp/tests/fixtures/data/conll2003.txt",
-    "validation_data_path": "allennlp/tests/fixtures/data/conll2003.txt",
+    "train_data_path": "test_fixtures/data/conll2003.txt",
+    "validation_data_path": "test_fixtures/data/conll2003.txt",
     "model": {
       "type": "simple_tagger",
       "text_field_embedder": {
@@ -24,8 +24,8 @@
               },
               "elmo": {
                 "type": "elmo_token_embedder",
-                "options_file": "allennlp/tests/fixtures/elmo/options.json",
-                "weight_file": "allennlp/tests/fixtures/elmo/lm_weights.hdf5"
+                "options_file": "test_fixtures/elmo/options.json",
+                "weight_file": "test_fixtures/elmo/lm_weights.hdf5"
               }
           }
       },