Windows testing support for Rasa #5784
Conversation
| if args.no_prompt: | ||
| should_train = True | ||
| else: | ||
| should_train = questionary.confirm( |
There was a problem hiding this comment.
what happened to the skip_if()s here?
There was a problem hiding this comment.
It simply doesn't work on Windows :( (it's an issue with questionary)
There was a problem hiding this comment.
@alwx can we fix that in the questionary package? do you know what the underlying issue is?
There was a problem hiding this comment.
Had no chance to check it yet. Will try to find time for it tomorrow, maybe it's an easy fix.
| def __init__( | ||
| self, | ||
| timestamp: Optional[float] = None, | ||
| metadata: Optional[Dict[Text, Any]] = None, | ||
| ) -> None: | ||
| super().__init__(timestamp, metadata) | ||
|
|
| @@ -17,6 +31,10 @@ def json_of_latest_request(r): | |||
| return r[-1].kwargs["json"] | |||
|
|
|||
|
|
|||
| def platform_independent_paths(coll: List[Text]): | |||
| return [i.replace("\\", "/") for i in coll] | |||
There was a problem hiding this comment.
I think we should be able to use pathlib.Path everywhere which converts to PureWindowsPath or PurePosixPath automatically depending on the platform
There was a problem hiding this comment.
in that case we could get rid of this function
| assert ( | ||
| utils.relative_normpath(test_file, "/my/test").replace("\\", "/") | ||
| == "path/file.txt" | ||
| ) |
There was a problem hiding this comment.
this should be platform-agnostic without the replace()
| assert ( | |
| utils.relative_normpath(test_file, "/my/test").replace("\\", "/") | |
| == "path/file.txt" | |
| ) | |
| assert ( | |
| assert Path(utils.relative_normpath(test_file, "/my/test")) == Path("path/file.txt") | |
| ) |
| @@ -32,7 +32,10 @@ def fake_model_dir(empty_model_dir): | |||
|
|
|||
| def test_relative_normpath(): | |||
There was a problem hiding this comment.
we could also update the utils function to use Path(p).resolve() - it replaces os.path.normpath() as of py 3.6: https://docs.python.org/3.6/library/pathlib.html#pathlib.Path.resolve
| core_files, nlu_files = data.get_core_nlu_files([data_dir]) | ||
| assert nlu_files == expected | ||
| paths = data.get_core_nlu_files([data_dir]) | ||
| assert platform_independent_paths(paths[1]) == expected |
There was a problem hiding this comment.
| assert platform_independent_paths(paths[1]) == expected | |
| assert Path(paths[1]) == Path(expected) |
| @@ -218,6 +218,7 @@ async def mocked_handle_message(self, message: UserMessage, wait: float) -> None | |||
| assert time.time() - start_time < time_limit | |||
|
|
|||
|
|
|||
| @pytest.mark.unix | |||
| BotUttered("Hi"), | ||
| SessionStarted(), | ||
| UserUttered("Ciao", {"name": "greet"}), | ||
| UserUttered("Hola", {"name": "greet"}, timestamp=1), |
There was a problem hiding this comment.
do we need this because the timestamps on Windows would be the same?
There was a problem hiding this comment.
yes, all the timestamps on windows are the same, and the order is not consistent
| @@ -207,6 +207,7 @@ def send_request() -> None: | |||
| train_request.terminate() | |||
|
|
|||
|
|
|||
| @pytest.mark.unix | |||
There was a problem hiding this comment.
why is this one required?
| import rasa.utils.io as io_utils | ||
|
|
||
| from yarl import URL |
There was a problem hiding this comment.
this is either the wrong import or it should be added to the dev dependencies
Co-authored-by: Ella Rohm-Ensing <erohmensing@gmail.com>
Co-authored-by: Ella Rohm-Ensing <erohmensing@gmail.com>
Co-authored-by: Ella Rohm-Ensing <erohmensing@gmail.com>
Proposed changes:
Status (please check what you already did):
black(please check Readme for instructions)