From 0c6be65f60b0f1c957b078b2e1f16ba573751ff0 Mon Sep 17 00:00:00 2001 From: alwx Date: Fri, 12 Nov 2021 15:36:32 +0100 Subject: [PATCH 1/9] `rasa data migrate` doesn't clean up if migration fails --- rasa/core/migrate.py | 100 ++++++++++++++++++++----------------- tests/core/test_migrate.py | 45 +++++++++++++++++ 2 files changed, 98 insertions(+), 47 deletions(-) diff --git a/rasa/core/migrate.py b/rasa/core/migrate.py index a19ca8ff8f2b..7cbaf40828c9 100644 --- a/rasa/core/migrate.py +++ b/rasa/core/migrate.py @@ -1,4 +1,5 @@ import copy +import shutil from pathlib import Path from typing import List, Dict, Text, Any, Tuple, Optional, Union @@ -180,6 +181,53 @@ def _write_final_domain( rasa.shared.utils.io.write_yaml(new_domain, out_file, True) +def _migrate_domain_files(domain_file: Path, backup_dir: Path, out_file: Path): + slots = {} + forms = {} + entities = [] + + for file in domain_file.iterdir(): + if not Domain.is_domain_file(file): + continue + + backup = backup_dir / file.name + original_content = _create_back_up(file, backup) + + if KEY_SLOTS not in original_content and KEY_FORMS not in original_content: + # this is done so that the other domain files can be moved + # in the migrated directory + rasa.shared.utils.io.write_yaml( + original_content, out_file / file.name, True + ) + elif KEY_SLOTS in original_content and slots: + raise RasaException( + f"Domain files with multiple '{KEY_SLOTS}' " + f"sections were provided. Please group these sections " + f"in one file only to prevent content duplication across " + f"multiple files. " + ) + elif KEY_FORMS in original_content and forms: + raise RasaException( + f"Domain files with multiple '{KEY_FORMS}' " + f"sections were provided. Please group these sections " + f"in one file only to prevent content duplication across " + f"multiple files. " + ) + + slots.update(original_content.get(KEY_SLOTS, {})) + forms.update(original_content.get(KEY_FORMS, {})) + entities.extend(original_content.get(KEY_ENTITIES, {})) + + if not slots or not forms: + raise RasaException( + f"The files you have provided in '{domain_file}' are missing slots " + f"or forms. Please make sure to include these for a " + f"successful migration." + ) + + return {KEY_SLOTS: slots, KEY_FORMS: forms, KEY_ENTITIES: entities} + + def migrate_domain_format( domain_file: Union[Text, Path], out_file: Union[Text, Path] ) -> None: @@ -193,11 +241,6 @@ def migrate_domain_format( backup_dir = current_dir / "original_domain" backup_dir.mkdir() - slots = {} - forms = {} - entities = [] - original_domain = {} - if out_file.is_file() or not out_file.exists(): out_file = current_dir / "new_domain" out_file.mkdir() @@ -207,48 +250,11 @@ def migrate_domain_format( f"for migrated domain files." ) - for file in domain_file.iterdir(): - if not Domain.is_domain_file(file): - continue - - backup = backup_dir / file.name - original_content = _create_back_up(file, backup) - - if KEY_SLOTS not in original_content and KEY_FORMS not in original_content: - # this is done so that the other domain files can be moved - # in the migrated directory - rasa.shared.utils.io.write_yaml( - original_content, out_file / file.name, True - ) - elif KEY_SLOTS in original_content and slots: - raise RasaException( - f"Domain files with multiple '{KEY_SLOTS}' " - f"sections were provided. Please group these sections " - f"in one file only to prevent content duplication across " - f"multiple files. " - ) - elif KEY_FORMS in original_content and forms: - raise RasaException( - f"Domain files with multiple '{KEY_FORMS}' " - f"sections were provided. Please group these sections " - f"in one file only to prevent content duplication across " - f"multiple files. " - ) - - slots.update(original_content.get(KEY_SLOTS, {})) - forms.update(original_content.get(KEY_FORMS, {})) - entities.extend(original_content.get(KEY_ENTITIES, {})) - - if not slots or not forms: - raise RasaException( - f"The files you have provided in '{domain_file}' are missing slots " - f"or forms. Please make sure to include these for a " - f"successful migration." - ) - - original_domain.update( - {KEY_SLOTS: slots, KEY_FORMS: forms, KEY_ENTITIES: entities} - ) + try: + original_domain = _migrate_domain_files(domain_file, backup_dir, out_file) + except Exception as e: + shutil.rmtree(backup_dir) + raise e else: if not Domain.is_domain_file(domain_file): raise RasaException( diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index 07371e638585..580d8f14f8c1 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -764,3 +764,48 @@ def test_migrate_domain_raises_for_non_domain_files(tmp_path: Path): f"Please make sure to include these for a successful migration.", ): rasa.core.migrate.migrate_domain_format(domain_dir, domain_dir) + + +def test_migrate_twice(tmp_path: Path,): + domain_dir = tmp_path / "domain" + domain_dir.mkdir() + + prepare_domain_path( + domain_dir, + """ + version: "2.0" + entities: + - outdoor + slots: + outdoor_seating: + type: bool + influence_conversation: false + """, + "slots_one.yml", + ) + + prepare_domain_path( + domain_dir, + """ + version: "2.0" + entities: + - outdoor + slots: + cuisine: + type: text + influence_conversation: false + """, + "slots_two.yml", + ) + + with pytest.raises( + RasaException, + match="Domain files with multiple 'slots' sections were provided.", + ): + rasa.core.migrate.migrate_domain_format(domain_dir, domain_dir) + + with pytest.raises( + RasaException, + match="Domain files with multiple 'slots' sections were provided.", + ): + rasa.core.migrate.migrate_domain_format(domain_dir, domain_dir) From 26ccbc300e27d4aa3d1cbf21de468e759457dfe9 Mon Sep 17 00:00:00 2001 From: alwx Date: Mon, 15 Nov 2021 09:20:01 +0100 Subject: [PATCH 2/9] Updated tests --- rasa/core/migrate.py | 8 +++++++- tests/core/test_migrate.py | 10 ++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/rasa/core/migrate.py b/rasa/core/migrate.py index 7cbaf40828c9..003934ee9ab8 100644 --- a/rasa/core/migrate.py +++ b/rasa/core/migrate.py @@ -181,7 +181,9 @@ def _write_final_domain( rasa.shared.utils.io.write_yaml(new_domain, out_file, True) -def _migrate_domain_files(domain_file: Path, backup_dir: Path, out_file: Path): +def _migrate_domain_files( + domain_file: Path, backup_dir: Path, out_file: Path +) -> Dict[Text, Any]: slots = {} forms = {} entities = [] @@ -234,6 +236,7 @@ def migrate_domain_format( """Converts 2.0 domain to 3.0 format.""" domain_file = Path(domain_file) out_file = Path(out_file) + created_out_dir = False current_dir = domain_file.parent @@ -244,6 +247,7 @@ def migrate_domain_format( if out_file.is_file() or not out_file.exists(): out_file = current_dir / "new_domain" out_file.mkdir() + created_out_dir = True rasa.shared.utils.io.raise_warning( f"The out path provided is not a directory, " f"creating a new directory '{str(out_file)}' " @@ -254,6 +258,8 @@ def migrate_domain_format( original_domain = _migrate_domain_files(domain_file, backup_dir, out_file) except Exception as e: shutil.rmtree(backup_dir) + if created_out_dir: + shutil.rmtree(out_file) raise e else: if not Domain.is_domain_file(domain_file): diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index 580d8f14f8c1..aef785d40851 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -1,4 +1,5 @@ import re +import os import textwrap from pathlib import Path from typing import Text, Any @@ -769,6 +770,7 @@ def test_migrate_domain_raises_for_non_domain_files(tmp_path: Path): def test_migrate_twice(tmp_path: Path,): domain_dir = tmp_path / "domain" domain_dir.mkdir() + migrated_domain_dir = tmp_path / "domain2" prepare_domain_path( domain_dir, @@ -802,10 +804,14 @@ def test_migrate_twice(tmp_path: Path,): RasaException, match="Domain files with multiple 'slots' sections were provided.", ): - rasa.core.migrate.migrate_domain_format(domain_dir, domain_dir) + rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) with pytest.raises( RasaException, match="Domain files with multiple 'slots' sections were provided.", ): - rasa.core.migrate.migrate_domain_format(domain_dir, domain_dir) + rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) + + current_dir = domain_dir.parent + assert not os.path.exists(current_dir / "original_domain") + assert not os.path.exists(current_dir / "new_domain") From b345a3c7c1334987a22133b38a64102702f2bf7d Mon Sep 17 00:00:00 2001 From: alwx Date: Mon, 15 Nov 2021 10:45:47 +0100 Subject: [PATCH 3/9] Tests for cleanup --- rasa/core/migrate.py | 4 +--- tests/core/test_migrate.py | 28 +++++++++++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/rasa/core/migrate.py b/rasa/core/migrate.py index 003934ee9ab8..3667135960dc 100644 --- a/rasa/core/migrate.py +++ b/rasa/core/migrate.py @@ -236,7 +236,6 @@ def migrate_domain_format( """Converts 2.0 domain to 3.0 format.""" domain_file = Path(domain_file) out_file = Path(out_file) - created_out_dir = False current_dir = domain_file.parent @@ -247,7 +246,6 @@ def migrate_domain_format( if out_file.is_file() or not out_file.exists(): out_file = current_dir / "new_domain" out_file.mkdir() - created_out_dir = True rasa.shared.utils.io.raise_warning( f"The out path provided is not a directory, " f"creating a new directory '{str(out_file)}' " @@ -258,7 +256,7 @@ def migrate_domain_format( original_domain = _migrate_domain_files(domain_file, backup_dir, out_file) except Exception as e: shutil.rmtree(backup_dir) - if created_out_dir: + if out_file != domain_file: shutil.rmtree(out_file) raise e else: diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index aef785d40851..27a3669ea459 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -1,5 +1,4 @@ import re -import os import textwrap from pathlib import Path from typing import Text, Any @@ -767,7 +766,7 @@ def test_migrate_domain_raises_for_non_domain_files(tmp_path: Path): rasa.core.migrate.migrate_domain_format(domain_dir, domain_dir) -def test_migrate_twice(tmp_path: Path,): +def test_migration_cleanup(tmp_path: Path,): domain_dir = tmp_path / "domain" domain_dir.mkdir() migrated_domain_dir = tmp_path / "domain2" @@ -804,7 +803,11 @@ def test_migrate_twice(tmp_path: Path,): RasaException, match="Domain files with multiple 'slots' sections were provided.", ): - rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) + rasa.core.migrate.migrate_domain_format(domain_dir, domain_dir) + + # here the migration fails but we check if the domain was + # not deleted in the process of cleanup + assert Path.exists(domain_dir) with pytest.raises( RasaException, @@ -812,6 +815,21 @@ def test_migrate_twice(tmp_path: Path,): ): rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) + # the migration fails again and we check if the cleanup was done successfully current_dir = domain_dir.parent - assert not os.path.exists(current_dir / "original_domain") - assert not os.path.exists(current_dir / "new_domain") + assert Path.exists(domain_dir) + assert not Path.exists(current_dir / "original_domain") + assert not Path.exists(current_dir / "new_domain") + + # create the `migrated_domain_dir` to use it instead of the default one + migrated_domain_dir.mkdir() + + with pytest.raises( + RasaException, + match="Domain files with multiple 'slots' sections were provided.", + ): + rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) + + # check if the cleanup was done successfully after the `migrated_domain_dir` was created + assert not Path.exists(current_dir / "original_domain") + assert not Path.exists(current_dir / "new_domain") From 957514e13fc9081c1d8dc6a0479999aca890f1b4 Mon Sep 17 00:00:00 2001 From: alwx Date: Mon, 15 Nov 2021 11:30:01 +0100 Subject: [PATCH 4/9] Do not remove the out_dir if it was created before --- rasa/core/migrate.py | 4 +++- tests/core/test_migrate.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/rasa/core/migrate.py b/rasa/core/migrate.py index 3667135960dc..cb35288b9c2b 100644 --- a/rasa/core/migrate.py +++ b/rasa/core/migrate.py @@ -236,6 +236,7 @@ def migrate_domain_format( """Converts 2.0 domain to 3.0 format.""" domain_file = Path(domain_file) out_file = Path(out_file) + created_out_dir = False current_dir = domain_file.parent @@ -246,6 +247,7 @@ def migrate_domain_format( if out_file.is_file() or not out_file.exists(): out_file = current_dir / "new_domain" out_file.mkdir() + created_out_dir = True rasa.shared.utils.io.raise_warning( f"The out path provided is not a directory, " f"creating a new directory '{str(out_file)}' " @@ -256,7 +258,7 @@ def migrate_domain_format( original_domain = _migrate_domain_files(domain_file, backup_dir, out_file) except Exception as e: shutil.rmtree(backup_dir) - if out_file != domain_file: + if out_file != domain_file and created_out_dir: shutil.rmtree(out_file) raise e else: diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index f1d19f5ec8de..6a208e59672b 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -831,5 +831,6 @@ def test_migration_cleanup(tmp_path: Path,): rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) # check if the cleanup was done successfully after the `migrated_domain_dir` was created + assert Path.exists(migrated_domain_dir) assert not Path.exists(current_dir / "original_domain") assert not Path.exists(current_dir / "new_domain") From a936b79f61a20b875f2a767d3e77483895e99036 Mon Sep 17 00:00:00 2001 From: alwx Date: Mon, 15 Nov 2021 11:31:52 +0100 Subject: [PATCH 5/9] Code style update --- tests/core/test_migrate.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index 6a208e59672b..a03f5183bf34 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -815,7 +815,8 @@ def test_migration_cleanup(tmp_path: Path,): ): rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) - # the migration fails again and we check if the cleanup was done successfully + # the migration fails again and we check + # if the cleanup was done successfully current_dir = domain_dir.parent assert Path.exists(domain_dir) assert not Path.exists(current_dir / "original_domain") @@ -830,7 +831,8 @@ def test_migration_cleanup(tmp_path: Path,): ): rasa.core.migrate.migrate_domain_format(domain_dir, migrated_domain_dir) - # check if the cleanup was done successfully after the `migrated_domain_dir` was created + # check if the cleanup was done successfully after + # the `migrated_domain_dir` was created assert Path.exists(migrated_domain_dir) assert not Path.exists(current_dir / "original_domain") assert not Path.exists(current_dir / "new_domain") From 6494c921c659f17f037de85e4c82542dacada682 Mon Sep 17 00:00:00 2001 From: alwx Date: Mon, 15 Nov 2021 11:55:58 +0100 Subject: [PATCH 6/9] Checking if the dir is empty --- tests/core/test_migrate.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index a03f5183bf34..cc76db707490 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -1,3 +1,4 @@ +import os import re import textwrap from pathlib import Path @@ -833,6 +834,6 @@ def test_migration_cleanup(tmp_path: Path,): # check if the cleanup was done successfully after # the `migrated_domain_dir` was created - assert Path.exists(migrated_domain_dir) + assert Path.exists(migrated_domain_dir) and len(os.listdir(migrated_domain_dir)) == 0 assert not Path.exists(current_dir / "original_domain") assert not Path.exists(current_dir / "new_domain") From 72e385252259aa2bdfb1f221a37e5827cca48d9b Mon Sep 17 00:00:00 2001 From: alwx Date: Tue, 16 Nov 2021 09:37:03 +0100 Subject: [PATCH 7/9] Updates & fixes --- rasa/core/migrate.py | 6 +++++- tests/core/test_migrate.py | 11 +++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/rasa/core/migrate.py b/rasa/core/migrate.py index cb35288b9c2b..ee0f7f738667 100644 --- a/rasa/core/migrate.py +++ b/rasa/core/migrate.py @@ -258,8 +258,12 @@ def migrate_domain_format( original_domain = _migrate_domain_files(domain_file, backup_dir, out_file) except Exception as e: shutil.rmtree(backup_dir) - if out_file != domain_file and created_out_dir: + if out_file != domain_file: shutil.rmtree(out_file) + if not created_out_dir: + # we recreate the deleted directory + # because it existed before + out_file.mkdir() raise e else: if not Domain.is_domain_file(domain_file): diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index cc76db707490..5778e1d15b4e 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -800,6 +800,17 @@ def test_migration_cleanup(tmp_path: Path,): "slots_two.yml", ) + prepare_domain_path( + domain_dir, + """ + version: "2.0" + responses: + utter_greet: + - text: "Hi there!" + """, + "responses.yml", + ) + with pytest.raises( RasaException, match="Domain files with multiple 'slots' sections were provided.", From ce2849f06e3fe00b112ed1e6eea4fc2d573ffa1f Mon Sep 17 00:00:00 2001 From: alwx Date: Tue, 16 Nov 2021 09:37:20 +0100 Subject: [PATCH 8/9] Code style update --- tests/core/test_migrate.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/core/test_migrate.py b/tests/core/test_migrate.py index 5778e1d15b4e..62d5e23a6c77 100644 --- a/tests/core/test_migrate.py +++ b/tests/core/test_migrate.py @@ -845,6 +845,8 @@ def test_migration_cleanup(tmp_path: Path,): # check if the cleanup was done successfully after # the `migrated_domain_dir` was created - assert Path.exists(migrated_domain_dir) and len(os.listdir(migrated_domain_dir)) == 0 + assert ( + Path.exists(migrated_domain_dir) and len(os.listdir(migrated_domain_dir)) == 0 + ) assert not Path.exists(current_dir / "original_domain") assert not Path.exists(current_dir / "new_domain") From 7d9282e0ac85224d3e2e574f4c28ed61eb92fd94 Mon Sep 17 00:00:00 2001 From: Lukas Osterloh Date: Wed, 10 Nov 2021 14:59:47 +0100 Subject: [PATCH 9/9] Improve validations around unused objects --- changelog/10079.bugfix.md | 1 + data/test_config/config_defaults.yml | 13 +++++++ data/test_validation/data/nlu.yml | 5 +++ data/test_validation/data/stories.yml | 6 ++++ data/test_validation/domain.yml | 12 +++++++ rasa/validator.py | 24 +++++++++---- tests/test_validator.py | 52 +++++++++++++++++++++++++++ 7 files changed, 107 insertions(+), 6 deletions(-) create mode 100644 changelog/10079.bugfix.md create mode 100644 data/test_validation/data/nlu.yml create mode 100644 data/test_validation/data/stories.yml create mode 100644 data/test_validation/domain.yml diff --git a/changelog/10079.bugfix.md b/changelog/10079.bugfix.md new file mode 100644 index 000000000000..05f915386fed --- /dev/null +++ b/changelog/10079.bugfix.md @@ -0,0 +1 @@ +Fixed validation behavior and logging output around unused intents and utterances. diff --git a/data/test_config/config_defaults.yml b/data/test_config/config_defaults.yml index 91a6056c1d99..6e9195534e75 100644 --- a/data/test_config/config_defaults.yml +++ b/data/test_config/config_defaults.yml @@ -24,3 +24,16 @@ pipeline: [] # ambiguity_threshold: 0.1 data: +policies: +# # No configuration for policies was provided. The following default policies were used to train your model. +# # If you'd like to customize them, uncomment and adjust the policies. +# # See https://rasa.com/docs/rasa/policies for more information. +# - name: MemoizationPolicy +# - name: RulePolicy +# - name: UnexpecTEDIntentPolicy +# max_history: 5 +# epochs: 100 +# - name: TEDPolicy +# max_history: 5 +# epochs: 100 +# constrain_similarities: true diff --git a/data/test_validation/data/nlu.yml b/data/test_validation/data/nlu.yml new file mode 100644 index 000000000000..0f66f3f7241a --- /dev/null +++ b/data/test_validation/data/nlu.yml @@ -0,0 +1,5 @@ +version: "2.0" +nlu: +- intent: greet + examples: | + - hey diff --git a/data/test_validation/data/stories.yml b/data/test_validation/data/stories.yml new file mode 100644 index 000000000000..235ba529df69 --- /dev/null +++ b/data/test_validation/data/stories.yml @@ -0,0 +1,6 @@ +version: "2.0" +stories: +- story: happy path + steps: + - intent: greet + - action: utter_greet diff --git a/data/test_validation/domain.yml b/data/test_validation/domain.yml new file mode 100644 index 000000000000..e72f350d6986 --- /dev/null +++ b/data/test_validation/domain.yml @@ -0,0 +1,12 @@ +version: "2.0" + +intents: + - greet + - goodbye + +responses: + utter_greet: + - text: "Hey! How are you?" + + utter_chatter: + - text: "Lovely weather today, isn't it?" diff --git a/rasa/validator.py b/rasa/validator.py index ab9e969c89fd..b497a4d33115 100644 --- a/rasa/validator.py +++ b/rasa/validator.py @@ -1,7 +1,7 @@ import itertools import logging from collections import defaultdict -from typing import Set, Text, Optional, Dict, Any +from typing import Set, Text, Optional, Dict, Any, List import rasa.core.training.story_conflict import rasa.shared.nlu.constants @@ -12,6 +12,7 @@ DOCS_URL_ACTIONS, REQUIRED_SLOTS_KEY, ) +from rasa.shared.core import constants from rasa.shared.core.constants import MAPPING_CONDITIONS, ACTIVE_LOOP from rasa.shared.core.domain import Domain from rasa.shared.core.events import ActionExecuted, ActiveLoop @@ -58,15 +59,22 @@ def from_importer(cls, importer: TrainingDataImporter) -> "Validator": return cls(domain, intents, story_graph, config) + def _non_default_intents(self) -> List[Text]: + return [ + item + for item in self.domain.intents + if item not in constants.DEFAULT_INTENTS + ] + def verify_intents(self, ignore_warnings: bool = True) -> bool: """Compares list of intents in domain with intents in NLU training data.""" everything_is_alright = True nlu_data_intents = {e.data["intent"] for e in self.intents.intent_examples} - for intent in self.domain.intents: + for intent in self._non_default_intents(): if intent not in nlu_data_intents: - logger.debug( + rasa.shared.utils.io.raise_warning( f"The intent '{intent}' is listed in the domain file, but " f"is not found in the NLU training data." ) @@ -134,9 +142,11 @@ def verify_intents_in_stories(self, ignore_warnings: bool = True) -> bool: ) everything_is_alright = False - for intent in self.domain.intents: + for intent in self._non_default_intents(): if intent not in stories_intents: - logger.debug(f"The intent '{intent}' is not used in any story.") + rasa.shared.utils.io.raise_warning( + f"The intent '{intent}' is not used in any story or rule." + ) everything_is_alright = ignore_warnings and everything_is_alright return everything_is_alright @@ -195,7 +205,9 @@ def verify_utterances_in_stories(self, ignore_warnings: bool = True) -> bool: for utterance in utterance_actions: if utterance not in stories_utterances: - logger.debug(f"The utterance '{utterance}' is not used in any story.") + rasa.shared.utils.io.raise_warning( + f"The utterance '{utterance}' is not used in any story or rule." + ) everything_is_alright = ignore_warnings and everything_is_alright return everything_is_alright diff --git a/tests/test_validator.py b/tests/test_validator.py index 33efa5e8d392..95a42d6beec8 100644 --- a/tests/test_validator.py +++ b/tests/test_validator.py @@ -1,6 +1,7 @@ from typing import Text import pytest +from _pytest.logging import LogCaptureFixture from rasa.validator import Validator from rasa.shared.importers.rasa import RasaFileImporter @@ -8,6 +9,19 @@ from pathlib import Path +@pytest.fixture(scope="class") +def validator_under_test() -> Validator: + importer = RasaFileImporter( + domain_path="data/test_validation/domain.yml", + training_data_paths=[ + "data/test_validation/data/nlu.yml", + "data/test_validation/data/stories.yml", + ], + ) + validator = Validator.from_importer(importer) + return validator + + def test_verify_nlu_with_e2e_story(tmp_path: Path, nlu_data_path: Path): story_file_name = tmp_path / "stories.yml" with open(story_file_name, "w") as file: @@ -213,6 +227,44 @@ def test_verify_there_is_example_repetition_in_intents(nlu_data_path: Text): assert not validator.verify_example_repetition_in_intents(False) +def test_verify_logging_message_for_intent_not_used_in_nlu( + caplog: LogCaptureFixture, validator_under_test: Validator +): + caplog.clear() + with pytest.warns(UserWarning) as record: + validator_under_test.verify_intents(False) + + assert ( + "The intent 'goodbye' is listed in the domain file, " + "but is not found in the NLU training data." + in (m.message.args[0] for m in record) + ) + + +def test_verify_logging_message_for_intent_not_used_in_story( + caplog: LogCaptureFixture, validator_under_test: Validator +): + caplog.clear() + with pytest.warns(UserWarning) as record: + validator_under_test.verify_intents_in_stories(False) + + assert "The intent 'goodbye' is not used in any story or rule." in ( + m.message.args[0] for m in record + ) + + +def test_verify_logging_message_for_unused_utterance( + caplog: LogCaptureFixture, validator_under_test: Validator +): + caplog.clear() + with pytest.warns(UserWarning) as record: + validator_under_test.verify_utterances_in_stories(False) + + assert "The utterance 'utter_chatter' is not used in any story or rule." in ( + m.message.args[0] for m in record + ) + + def test_verify_logging_message_for_repetition_in_intents(caplog, nlu_data_path: Text): # moodbot nlu data already has duplicated example 'good afternoon' # for intents greet and goodbye