From ce659e04eb756b802623065f47aa99b3a5f81fe1 Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Mon, 14 May 2018 10:04:16 -0700 Subject: [PATCH 01/10] Add allennlp test-install command --- allennlp/commands/__init__.py | 3 +- allennlp/commands/test_install.py | 58 +++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 allennlp/commands/test_install.py diff --git a/allennlp/commands/__init__.py b/allennlp/commands/__init__.py index 92ce75e9782..60aaba3bd70 100644 --- a/allennlp/commands/__init__.py +++ b/allennlp/commands/__init__.py @@ -11,6 +11,7 @@ from allennlp.commands.serve import Serve from allennlp.commands.dry_run import DryRun from allennlp.commands.subcommand import Subcommand +from allennlp.commands.test_install import TestInstall from allennlp.commands.train import Train from allennlp.service.predictors import DemoModel from allennlp.common.util import import_submodules @@ -40,7 +41,7 @@ def main(prog: str = None, "elmo": Elmo(), "fine-tune": FineTune(), "dry-run": DryRun(), - + "test-install": TestInstall(), # Superseded by overrides **subcommand_overrides } diff --git a/allennlp/commands/test_install.py b/allennlp/commands/test_install.py new file mode 100644 index 00000000000..335bb399adb --- /dev/null +++ b/allennlp/commands/test_install.py @@ -0,0 +1,58 @@ +""" +The ``test-install`` subcommand verifies +an installation by running the unit tests. + +.. code-block:: bash + + $ allennlp test-install --help + usage: allennlp test-install [-h] [--run-all] + [--include-package INCLUDE_PACKAGE] + + Test that installation works by running the unit tests. + + optional arguments: + -h, --help show this help message and exit + --run-all By default, we skip tests that are slow or download + large files. This flag will run all tests. + --include-package INCLUDE_PACKAGE + additional packages to include +""" + +import argparse +import logging +import os + +from allennlp.commands.subcommand import Subcommand +import pytest + +logger = logging.getLogger(__name__) # pylint: disable=invalid-name + +class TestInstall(Subcommand): + def add_subparser(self, name: str, parser: argparse._SubParsersAction) -> argparse.ArgumentParser: + # pylint: disable=protected-access + description = '''Test that installation works by running the unit tests.''' + subparser = parser.add_parser( + 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.set_defaults(func=_run_test) + + return subparser + + +def _run_test(args: argparse.Namespace): + project_root = os.path.abspath( + os.path.join( + os.path.dirname(os.path.realpath(__file__)), + os.pardir, os.pardir)) + logger.info("Changing directory to {}".format(project_root)) + os.chdir(project_root) + test_dir = os.path.join(project_root, "tests") + logger.info("Running tests at {}".format(test_dir)) + if args.run_all: + pytest.main([test_dir]) + else: + pytest.main([test_dir, '-k', 'not sniff_test']) From 737c46d6b521c9495562c7f8d8c22e43ad274a4f Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Mon, 14 May 2018 10:16:36 -0700 Subject: [PATCH 02/10] Add back a newline that I accidentally removed --- allennlp/commands/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/allennlp/commands/__init__.py b/allennlp/commands/__init__.py index 60aaba3bd70..ec595a0c0bc 100644 --- a/allennlp/commands/__init__.py +++ b/allennlp/commands/__init__.py @@ -42,6 +42,7 @@ def main(prog: str = None, "fine-tune": FineTune(), "dry-run": DryRun(), "test-install": TestInstall(), + # Superseded by overrides **subcommand_overrides } From 332d51f8e0533b911b81f36ad1df59bbad6f1223 Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Mon, 14 May 2018 10:39:49 -0700 Subject: [PATCH 03/10] Add small test for _get_project_root --- allennlp/commands/test_install.py | 22 +++++++++++++++------- tests/commands/test_install_test.py | 13 +++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) create mode 100644 tests/commands/test_install_test.py diff --git a/allennlp/commands/test_install.py b/allennlp/commands/test_install.py index 335bb399adb..4a3e483b038 100644 --- a/allennlp/commands/test_install.py +++ b/allennlp/commands/test_install.py @@ -22,9 +22,10 @@ import logging import os -from allennlp.commands.subcommand import Subcommand import pytest +from allennlp.commands.subcommand import Subcommand + logger = logging.getLogger(__name__) # pylint: disable=invalid-name class TestInstall(Subcommand): @@ -43,16 +44,23 @@ def add_subparser(self, name: str, parser: argparse._SubParsersAction) -> argpar return subparser +def _get_project_root(): + return os.path.abspath( + os.path.join( + os.path.dirname(os.path.realpath(__file__)), + os.pardir, os.pardir)) + + def _run_test(args: argparse.Namespace): - project_root = os.path.abspath( - os.path.join( - os.path.dirname(os.path.realpath(__file__)), - os.pardir, os.pardir)) - logger.info("Changing directory to {}".format(project_root)) + initial_working_dir = os.getcwd() + project_root = _get_project_root() + logger.info("Changing directory to %s", project_root) os.chdir(project_root) test_dir = os.path.join(project_root, "tests") - logger.info("Running tests at {}".format(test_dir)) + logger.info("Running tests at %s", test_dir) if args.run_all: pytest.main([test_dir]) else: pytest.main([test_dir, '-k', 'not sniff_test']) + # Change back to original working directory after running tests + os.chdir(initial_working_dir) diff --git a/tests/commands/test_install_test.py b/tests/commands/test_install_test.py new file mode 100644 index 00000000000..d1dd3a164ac --- /dev/null +++ b/tests/commands/test_install_test.py @@ -0,0 +1,13 @@ +# pylint: disable=invalid-name,no-self-use +import os + +from allennlp.common.testing import AllenNlpTestCase +from allennlp.commands.test_install import _get_project_root + + +class TestTestInstall(AllenNlpTestCase): + def test_get_project_root(self): + project_root = _get_project_root() + assert os.path.exists(os.path.join(project_root), "tests") + assert os.path.exists(os.path.join(project_root), "LICENSE") + assert os.path.exists(os.path.join(project_root), "setup.py") From 39c42b315f407d2abc362cb591ed4b63edaef83d Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Mon, 14 May 2018 11:09:40 -0700 Subject: [PATCH 04/10] Fix typo in parens --- tests/commands/test_install_test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/commands/test_install_test.py b/tests/commands/test_install_test.py index d1dd3a164ac..50e71bb2d11 100644 --- a/tests/commands/test_install_test.py +++ b/tests/commands/test_install_test.py @@ -7,7 +7,8 @@ class TestTestInstall(AllenNlpTestCase): def test_get_project_root(self): + # pylint: disable=C0321 project_root = _get_project_root() - assert os.path.exists(os.path.join(project_root), "tests") - assert os.path.exists(os.path.join(project_root), "LICENSE") - assert os.path.exists(os.path.join(project_root), "setup.py") + assert os.path.exists(os.path.join(project_root, "tests")) + assert os.path.exists(os.path.join(project_root, "LICENSE")) + assert os.path.exists(os.path.join(project_root, "setup.py")) From 47925daa2658782168263c213480af4f0416e91b Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Mon, 14 May 2018 11:10:36 -0700 Subject: [PATCH 05/10] Remove erroneous copy/paste --- tests/commands/test_install_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/commands/test_install_test.py b/tests/commands/test_install_test.py index 50e71bb2d11..0fb56546a8a 100644 --- a/tests/commands/test_install_test.py +++ b/tests/commands/test_install_test.py @@ -7,7 +7,6 @@ class TestTestInstall(AllenNlpTestCase): def test_get_project_root(self): - # pylint: disable=C0321 project_root = _get_project_root() assert os.path.exists(os.path.join(project_root, "tests")) assert os.path.exists(os.path.join(project_root, "LICENSE")) From 7087faf7b76345679d6d737b8203aa65e600f0f0 Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Mon, 14 May 2018 11:30:59 -0700 Subject: [PATCH 06/10] Add docs for test-install --- doc/api/allennlp.commands.rst | 20 ++++++++++++-------- doc/api/allennlp.commands.test_install.rst | 4 ++++ 2 files changed, 16 insertions(+), 8 deletions(-) create mode 100644 doc/api/allennlp.commands.test_install.rst diff --git a/doc/api/allennlp.commands.rst b/doc/api/allennlp.commands.rst index 2761bbecdd4..865c97ac806 100644 --- a/doc/api/allennlp.commands.rst +++ b/doc/api/allennlp.commands.rst @@ -17,14 +17,17 @@ The included module ``allennlp.run`` is such a script: -h, --help show this help message and exit Commands: - train Train a model - evaluate Evaluate the specified model + dataset - predict Use a trained model to make predictions. - serve Run the web service and demo. - make-vocab - Create a vocabulary - elmo Use a trained model to make predictions. - dry-run Create a vocabulary, compute dataset statistics and other training utilities. + train Train a model + evaluate Evaluate the specified model + dataset + predict Use a trained model to make predictions. + serve Run the web service and demo. + make-vocab Create a vocabulary + elmo Use a trained model to make predictions. + fine-tune Continue training a model on a new dataset + dry-run Create a vocabulary, compute dataset statistics and other + training utilities. + test-install + Run the unit tests. However, it only knows about the models and classes that are included with AllenNLP. Once you start creating custom models, @@ -41,6 +44,7 @@ calls ``main()``. allennlp.commands.fine_tune allennlp.commands.elmo allennlp.commands.dry_run + allennlp.commands.test_install .. automodule:: allennlp.commands :members: diff --git a/doc/api/allennlp.commands.test_install.rst b/doc/api/allennlp.commands.test_install.rst new file mode 100644 index 00000000000..40fda6b8687 --- /dev/null +++ b/doc/api/allennlp.commands.test_install.rst @@ -0,0 +1,4 @@ +allennlp.commands.test_install +============================== + +.. automodule:: allennlp.commands.test_install From bb40f30e96679deadd176c7d7f8512b25b66a712 Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Wed, 16 May 2018 14:01:20 -0700 Subject: [PATCH 07/10] Remove some unused imports in commands.__init__.py --- allennlp/commands/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/allennlp/commands/__init__.py b/allennlp/commands/__init__.py index ec595a0c0bc..7922b3fe31c 100644 --- a/allennlp/commands/__init__.py +++ b/allennlp/commands/__init__.py @@ -1,7 +1,6 @@ from typing import Dict import argparse import logging -import sys from allennlp.commands.elmo import Elmo from allennlp.commands.evaluate import Evaluate @@ -13,7 +12,6 @@ from allennlp.commands.subcommand import Subcommand from allennlp.commands.test_install import TestInstall from allennlp.commands.train import Train -from allennlp.service.predictors import DemoModel from allennlp.common.util import import_submodules logger = logging.getLogger(__name__) # pylint: disable=invalid-name From 334108fde07920bd9b8f8d9a1afbc43201be6a3c Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Wed, 16 May 2018 14:03:11 -0700 Subject: [PATCH 08/10] Turn off notebook tests by default as well --- allennlp/commands/test_install.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/allennlp/commands/test_install.py b/allennlp/commands/test_install.py index 4a3e483b038..03421d49ef5 100644 --- a/allennlp/commands/test_install.py +++ b/allennlp/commands/test_install.py @@ -61,6 +61,6 @@ def _run_test(args: argparse.Namespace): if args.run_all: pytest.main([test_dir]) else: - pytest.main([test_dir, '-k', 'not sniff_test']) + pytest.main([test_dir, '-k', 'not sniff_test and not notebooks_test']) # Change back to original working directory after running tests os.chdir(initial_working_dir) From ab6d8df4b2fb1989ab79321415987eb28d01a61c Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Wed, 16 May 2018 14:08:38 -0700 Subject: [PATCH 09/10] Raise informative error message if jupyter not found --- tests/notebooks_test.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/notebooks_test.py b/tests/notebooks_test.py index 9d23a76439e..4d999b5dc19 100644 --- a/tests/notebooks_test.py +++ b/tests/notebooks_test.py @@ -1,8 +1,12 @@ import os -import nbformat -from nbconvert.preprocessors.execute import CellExecutionError -from nbconvert.preprocessors import ExecutePreprocessor +try: + import nbformat + from nbconvert.preprocessors.execute import CellExecutionError + from nbconvert.preprocessors import ExecutePreprocessor +except ModuleNotFoundError: + print("jupyter must be installed in order to run notebook tests. " + "To install with pip, run: pip install jupyter") from allennlp.common.testing import AllenNlpTestCase From 414c757768b0d574c7e170c81d1383572b195bd4 Mon Sep 17 00:00:00 2001 From: Nelson Liu Date: Wed, 16 May 2018 14:10:41 -0700 Subject: [PATCH 10/10] Move pytest, flaky, responses to requirements --- requirements.txt | 11 +++++++++++ requirements_test.txt | 10 ---------- setup.py | 5 ++++- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/requirements.txt b/requirements.txt index b0cc945fe0a..8df25db27c7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -66,3 +66,14 @@ h5py # For timezone utilities pytz==2017.3 + +#### ESSENTIAL TESTING-RELATED PACKAGES #### +# We'll use pytest to run our tests; this isn't really necessary to run the code, but it is to run +# the tests. With this here, you can run the tests with `py.test` from the base directory. +pytest + +# Allows marking tests as flaky, to be rerun if they fail +flaky + +# Required to mock out `requests` calls +responses>=0.7 diff --git a/requirements_test.txt b/requirements_test.txt index 3905175b992..403f6be02ed 100644 --- a/requirements_test.txt +++ b/requirements_test.txt @@ -3,10 +3,6 @@ # Checks style, syntax, and other useful errors. pylint==1.8.1 -# We'll use pytest to run our tests; this isn't really necessary to run the code, but it is to run -# the tests. With this here, you can run the tests with `py.test` from the base directory. -pytest - # Tutorial notebooks jupyter @@ -16,9 +12,6 @@ mypy==0.521 # Allows generation of coverage reports with pytest. pytest-cov -# Allows marking tests as flaky, to be rerun if they fail -flaky - # Allows codecov to generate coverage reports coverage codecov @@ -26,9 +19,6 @@ codecov # Required to run sanic tests aiohttp -# Required to mock out `requests` calls -responses>=0.7 - #### DOC-RELATED PACKAGES #### # Builds our documentation. diff --git a/setup.py b/setup.py index 8335eed245f..7341ecac9c0 100644 --- a/setup.py +++ b/setup.py @@ -120,7 +120,10 @@ 'scikit-learn', 'scipy', 'pytz==2017.3', - 'unidecode' + 'unidecode', + 'pytest', + 'flaky', + 'responses>=0.7' ], scripts=["bin/allennlp"], setup_requires=setup_requirements,