From f7f2f4108d626710651207f9dbc5b23548c76506 Mon Sep 17 00:00:00 2001 From: Federico Tedin Date: Mon, 25 Nov 2019 16:52:45 +0100 Subject: [PATCH 1/6] Fix tests for temp files. --- tests/cli/test_rasa_train.py | 31 --------------------------- tests/test_train.py | 41 ++++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 49 deletions(-) diff --git a/tests/cli/test_rasa_train.py b/tests/cli/test_rasa_train.py index 0a434a6e448a..1839c77b3ab9 100644 --- a/tests/cli/test_rasa_train.py +++ b/tests/cli/test_rasa_train.py @@ -263,31 +263,6 @@ def test_train_core_no_domain_exists(run_in_default_project: Callable[..., RunRe assert not os.path.isfile("train_rasa_models_no_domain/rasa-model.tar.gz") -def count_rasa_temp_files() -> int: - count = 0 - for entry in os.scandir(tempfile.gettempdir()): - if not entry.is_dir(): - continue - - try: - for f in os.listdir(entry.path): - if f.endswith("_nlu.md") or f.endswith("_stories.md"): - count += 1 - except PermissionError: - # Ignore permission errors - pass - - return count - - -def test_train_core_temp_files( - run_in_default_project: Callable[..., RunResult] -) -> None: - count = count_rasa_temp_files() - run_in_default_project("train", "core") - assert count == count_rasa_temp_files() - - def test_train_nlu(run_in_default_project: Callable[..., RunResult]): run_in_default_project( "train", @@ -341,12 +316,6 @@ def test_train_nlu_persist_nlu_data( ) -def test_train_nlu_temp_files(run_in_default_project: Callable[..., RunResult]): - count = count_rasa_temp_files() - run_in_default_project("train", "nlu") - assert count == count_rasa_temp_files() - - def test_train_help(run): output = run("train", "--help") diff --git a/tests/test_train.py b/tests/test_train.py index eccdf3b2d4fc..f1adf9c03c8b 100644 --- a/tests/test_train.py +++ b/tests/test_train.py @@ -1,16 +1,17 @@ import tempfile import os import shutil +from typing import Text import pytest +from _pytest.monkeypatch import MonkeyPatch +from _pytest.tmpdir import TempdirFactory import rasa.model from rasa.train import train from tests.core.test_model import _fingerprint -TEST_TEMP = "test_tmp" - @pytest.mark.parametrize( "parameters", @@ -45,24 +46,28 @@ def test_package_model(trained_rasa_model, parameters): assert file_name.endswith(".tar.gz") -@pytest.fixture -def move_tempdir(): - # Create a new *empty* tmp directory - shutil.rmtree(TEST_TEMP, ignore_errors=True) - os.mkdir(TEST_TEMP) - tempfile.tempdir = TEST_TEMP - yield - tempfile.tempdir = None - shutil.rmtree(TEST_TEMP) +def count_temp_rasa_files(directory: Text) -> int: + return len([ + entry for entry in os.listdir(directory) + if not any([ + # Ignore the following files/directories: + entry == "__pycache__", # Python bytecode + entry.endswith(".py") # Temp .py files created by TF + # Anything else is considered to be created by Rasa + ]) + ]) def test_train_temp_files( - move_tempdir, - default_domain_path, - default_stories_file, - default_stack_config, - default_nlu_data, + tmp_path: Text, + monkeypatch: MonkeyPatch, + default_domain_path: Text, + default_stories_file: Text, + default_stack_config: Text, + default_nlu_data: Text, ): + monkeypatch.setattr(tempfile, "tempdir", tmp_path) + train( default_domain_path, default_stack_config, @@ -70,7 +75,7 @@ def test_train_temp_files( force_training=True, ) - assert len(os.listdir(TEST_TEMP)) == 0 + assert count_temp_rasa_files(tempfile.tempdir) == 0 # After training the model, try to do it again. This shouldn't try to train # a new model because nothing has been changed. It also shouldn't create @@ -81,4 +86,4 @@ def test_train_temp_files( [default_stories_file, default_nlu_data], ) - assert len(os.listdir(TEST_TEMP)) == 0 + assert count_temp_rasa_files(tempfile.tempdir) == 0 From 070a3f7f3d3c5be0b6c77d15aa90c57c94e83a3a Mon Sep 17 00:00:00 2001 From: Federico Tedin Date: Mon, 25 Nov 2019 16:55:00 +0100 Subject: [PATCH 2/6] Black formatting. --- tests/test_train.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/test_train.py b/tests/test_train.py index f1adf9c03c8b..e54e302c5294 100644 --- a/tests/test_train.py +++ b/tests/test_train.py @@ -47,15 +47,20 @@ def test_package_model(trained_rasa_model, parameters): def count_temp_rasa_files(directory: Text) -> int: - return len([ - entry for entry in os.listdir(directory) - if not any([ - # Ignore the following files/directories: - entry == "__pycache__", # Python bytecode - entry.endswith(".py") # Temp .py files created by TF - # Anything else is considered to be created by Rasa - ]) - ]) + return len( + [ + entry + for entry in os.listdir(directory) + if not any( + [ + # Ignore the following files/directories: + entry == "__pycache__", # Python bytecode + entry.endswith(".py") # Temp .py files created by TF + # Anything else is considered to be created by Rasa + ] + ) + ] + ) def test_train_temp_files( From 2d8c5aa6aafb09f987dd8b2004f83010c43c58a4 Mon Sep 17 00:00:00 2001 From: Federico Tedin Date: Mon, 25 Nov 2019 17:17:08 +0100 Subject: [PATCH 3/6] Add temp tests for train_core and train_nlu. --- tests/test_train.py | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/tests/test_train.py b/tests/test_train.py index e54e302c5294..dfe9acf2bca3 100644 --- a/tests/test_train.py +++ b/tests/test_train.py @@ -9,7 +9,7 @@ import rasa.model -from rasa.train import train +from rasa.train import train_core, train_nlu, train from tests.core.test_model import _fingerprint @@ -72,11 +72,13 @@ def test_train_temp_files( default_nlu_data: Text, ): monkeypatch.setattr(tempfile, "tempdir", tmp_path) + output = "test_train_temp_files_models" train( default_domain_path, default_stack_config, [default_stories_file, default_nlu_data], + output=output, force_training=True, ) @@ -89,6 +91,45 @@ def test_train_temp_files( default_domain_path, default_stack_config, [default_stories_file, default_nlu_data], + output=output, + ) + + assert count_temp_rasa_files(tempfile.tempdir) == 0 + + +def test_train_core_temp_files( + tmp_path: Text, + monkeypatch: MonkeyPatch, + default_domain_path: Text, + default_stories_file: Text, + default_stack_config: Text, +): + monkeypatch.setattr(tempfile, "tempdir", tmp_path) + + train_core( + default_domain_path, + default_stack_config, + default_stories_file, + output="test_train_core_temp_files_models", + ) + + assert count_temp_rasa_files(tempfile.tempdir) == 0 + + +def test_train_nlu_temp_files( + tmp_path: Text, + monkeypatch: MonkeyPatch, + default_domain_path: Text, + default_stories_file: Text, + default_stack_config: Text, + default_nlu_data: Text, +): + monkeypatch.setattr(tempfile, "tempdir", tmp_path) + + train_nlu( + default_stack_config, + default_nlu_data, + output="test_train_nlu_temp_files_models", ) assert count_temp_rasa_files(tempfile.tempdir) == 0 From 6436566da87fefbf956b89a09f66ba551cc23881 Mon Sep 17 00:00:00 2001 From: Federico Tedin Date: Mon, 25 Nov 2019 17:19:40 +0100 Subject: [PATCH 4/6] Remove unused args. --- tests/test_train.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_train.py b/tests/test_train.py index dfe9acf2bca3..af9dc02412f5 100644 --- a/tests/test_train.py +++ b/tests/test_train.py @@ -119,8 +119,6 @@ def test_train_core_temp_files( def test_train_nlu_temp_files( tmp_path: Text, monkeypatch: MonkeyPatch, - default_domain_path: Text, - default_stories_file: Text, default_stack_config: Text, default_nlu_data: Text, ): From e48659decb9d171dfe4a54c411a90d70932875fb Mon Sep 17 00:00:00 2001 From: Federico Tedin Date: Mon, 25 Nov 2019 19:05:28 +0100 Subject: [PATCH 5/6] Clean up tests. - Use JOBS in makefile to specify number of workers (not -j). - Remove invalid @utilities.slowtest decorator. --- .travis.yml | 2 +- Makefile | 12 +++++------- tests/nlu/base/test_interpreter.py | 1 - tests/nlu/training/test_train.py | 4 ---- tests/nlu/utilities.py | 2 -- 5 files changed, 6 insertions(+), 15 deletions(-) diff --git a/.travis.yml b/.travis.yml index c4b097a5b794..8f779af30939 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,7 +42,7 @@ jobs: name: "Test 3.6" python: "3.6" script: - - make test -j 4 + - make test - <<: *run-tests name: "Test 3.7" python: '3.7' diff --git a/Makefile b/Makefile index 18f1c496785a..1e9bd7876a99 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,9 @@ .PHONY: clean test lint init check-readme -JOBS ?= 1 +JOBS ?= auto help: + @echo "make" @echo " clean" @echo " Remove Python/build artifacts." @echo " formatter" @@ -19,6 +20,7 @@ help: @echo " Download all additional project files needed to run tests." @echo " test" @echo " Run pytest on tests/." + @echo " Use the JOBS environment variable to configure number of workers (default: auto)." @echo " check-readme" @echo " Check if the README can be converted from .md to .rst for PyPI." @echo " doctest" @@ -46,7 +48,7 @@ types: pytype --keep-going rasa prepare-tests-macos: prepare-tests-files - brew install graphviz wget + brew install graphviz wget || true prepare-tests-ubuntu: prepare-tests-files sudo apt-get -y install graphviz graphviz-dev python3-tk @@ -58,14 +60,10 @@ prepare-tests-files: python -m spacy link de_core_news_sm de --force wget --progress=dot:giga -N -P data/ https://s3-eu-west-1.amazonaws.com/mitie/total_word_feature_extractor.dat -test: clean get-num-jobs +test: clean # OMP_NUM_THREADS can improve overral performance using one thread by process (on tensorflow), avoiding overload OMP_NUM_THREADS=1 pytest tests -n $(JOBS) --cov rasa -get-num-jobs: - $(eval JOBS := $(if $(findstring -j, $(MAKEFLAGS)), $(shell echo $(MAKEFLAGS) | sed -E "s@.*-j([0-9]+).*@\1@"), $(JOBS))) - $(eval JOBS := $(if $(findstring -j, $(JOBS)), auto, $(JOBS))) - doctest: clean cd docs && make doctest diff --git a/tests/nlu/base/test_interpreter.py b/tests/nlu/base/test_interpreter.py index 72187f43a063..186469298c89 100644 --- a/tests/nlu/base/test_interpreter.py +++ b/tests/nlu/base/test_interpreter.py @@ -15,7 +15,6 @@ from tests.nlu import utilities -@utilities.slowtest @pytest.mark.parametrize( "pipeline_template", list(registry.registered_pipeline_templates.keys()) ) diff --git a/tests/nlu/training/test_train.py b/tests/nlu/training/test_train.py index 725370d65364..dba73ef076f3 100644 --- a/tests/nlu/training/test_train.py +++ b/tests/nlu/training/test_train.py @@ -74,7 +74,6 @@ def test_all_components_are_in_at_least_one_test_pipeline(): ), "`all_components` template is missing component." -@utilities.slowtest @pytest.mark.parametrize( "pipeline_template", list(registry.registered_pipeline_templates.keys()) ) @@ -93,7 +92,6 @@ async def test_train_model(pipeline_template, component_builder, tmpdir): assert loaded.parse("Hello today is Monday, again!") is not None -@utilities.slowtest async def test_random_seed(component_builder, tmpdir): """test if train result is the same for two runs of tf embedding""" @@ -121,7 +119,6 @@ async def test_random_seed(component_builder, tmpdir): assert result_a == result_b -@utilities.slowtest @pytest.mark.parametrize("language, pipeline", pipelines_for_tests()) async def test_train_model_on_test_pipelines( language, pipeline, component_builder, tmpdir @@ -140,7 +137,6 @@ async def test_train_model_on_test_pipelines( assert loaded.parse("Hello today is Monday, again!") is not None -@utilities.slowtest @pytest.mark.parametrize("language, pipeline", pipelines_for_tests()) async def test_train_model_no_events(language, pipeline, component_builder, tmpdir): _config = RasaNLUModelConfig({"pipeline": pipeline, "language": language}) diff --git a/tests/nlu/utilities.py b/tests/nlu/utilities.py index 2b0de688c8fa..6cf509ee435d 100644 --- a/tests/nlu/utilities.py +++ b/tests/nlu/utilities.py @@ -7,8 +7,6 @@ from rasa.nlu.model import Interpreter from rasa.nlu.train import train -slowtest = pytest.mark.slowtest - def base_test_conf(pipeline_template): # 'response_log': temp_log_file_dir(), From 9487fadd60300f9186dfdb164fc6570db1dabd90 Mon Sep 17 00:00:00 2001 From: Federico Tedin Date: Mon, 25 Nov 2019 22:54:06 +0100 Subject: [PATCH 6/6] Use one worker by default for testing. --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 1e9bd7876a99..02ecd32d3388 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ .PHONY: clean test lint init check-readme -JOBS ?= auto +JOBS ?= 1 help: @echo "make" @@ -20,7 +20,7 @@ help: @echo " Download all additional project files needed to run tests." @echo " test" @echo " Run pytest on tests/." - @echo " Use the JOBS environment variable to configure number of workers (default: auto)." + @echo " Use the JOBS environment variable to configure number of workers (default: 1)." @echo " check-readme" @echo " Check if the README can be converted from .md to .rst for PyPI." @echo " doctest"