From f2a1863f2cc12dff8b7a324ad0edd3d31f7813f7 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Sat, 21 Sep 2019 22:40:41 +0600 Subject: [PATCH 1/4] Add Better errror message for lists in config file --- readthedocs/config/config.py | 6 +++++- readthedocs/config/tests/test_config.py | 14 +++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index eb745d03c7f..5c29c55b7bf 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -110,7 +110,11 @@ class InvalidConfig(ConfigError): message_template = 'Invalid "{key}": {error}' def __init__(self, key, code, error_message, source_file=None): - self.key = key + # Checks for patterns similar to `python.install.0.requirements` + # if matched change to `python.install[0].requirements` using backreference. + self.key = re.sub( + r'^(python\.install)(\.)(\d+)(\.\w+)$', r'\1[\3]\4', key + ) self.code = code self.source_file = source_file message = self.message_template.format( diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 5769b5ceb8d..394c9203d2f 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1062,7 +1062,7 @@ def test_python_install_method_check_invalid(self, value, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install.0.method' + assert excinfo.value.key == 'python.install[0].method' def test_python_install_requirements_check_valid(self, tmpdir): apply_fs(tmpdir, {'requirements.txt': ''}) @@ -1096,7 +1096,7 @@ def test_python_install_requirements_does_not_allow_null(self, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install.0.requirements' + assert excinfo.value.key == 'python.install[0].requirements' def test_python_install_requirements_does_not_allow_empty_string(self, tmpdir): build = self.get_build_config( @@ -1112,7 +1112,7 @@ def test_python_install_requirements_does_not_allow_empty_string(self, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install.0.requirements' + assert excinfo.value.key == 'python.install[0].requirements' def test_python_install_requirements_ignores_default(self, tmpdir): apply_fs(tmpdir, {'requirements.txt': ''}) @@ -1157,7 +1157,7 @@ def test_python_install_requirements_check_invalid_types(self, value, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install.0.requirements' + assert excinfo.value.key == 'python.install[0].requirements' def test_python_install_path_is_required(self, tmpdir): build = self.get_build_config( @@ -1307,7 +1307,7 @@ def test_python_install_extra_requirements_and_setuptools(self, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install.0.extra_requirements' + assert excinfo.value.key == 'python.install[0].extra_requirements' @pytest.mark.parametrize('value', [2, 'invalid', {}, '', None]) def test_python_install_extra_requirements_check_type(self, value, tmpdir): @@ -1325,7 +1325,7 @@ def test_python_install_extra_requirements_check_type(self, value, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install.0.extra_requirements' + assert excinfo.value.key == 'python.install[0].extra_requirements' def test_python_install_extra_requirements_allow_empty(self, tmpdir): build = self.get_build_config( @@ -1852,7 +1852,7 @@ def test_submodules_recursive_explicit_default(self): }] } }, - 'python.install.1.invalid' + 'python.install[1].invalid' ) ]) def test_strict_validation(self, value, key): From 3278ca830efa94eb55e6195c50ca109116b7d323 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 25 Sep 2019 00:20:06 +0600 Subject: [PATCH 2/4] msg regex moved to method --- readthedocs/config/config.py | 16 ++++++++++------ readthedocs/config/tests/test_config.py | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 5c29c55b7bf..c1c02561a8a 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -110,20 +110,24 @@ class InvalidConfig(ConfigError): message_template = 'Invalid "{key}": {error}' def __init__(self, key, code, error_message, source_file=None): - # Checks for patterns similar to `python.install.0.requirements` - # if matched change to `python.install[0].requirements` using backreference. - self.key = re.sub( - r'^(python\.install)(\.)(\d+)(\.\w+)$', r'\1[\3]\4', key - ) + self.key = self._get_display_key(key) self.code = code self.source_file = source_file message = self.message_template.format( - key=key, + key=self.key, code=code, error=error_message, ) super().__init__(message, code=code) + @staticmethod + def _get_display_key(key): + # Checks for patterns similar to `python.install.0.requirements` + # if matched change to `python.install[0].requirements` using backreference. + return re.sub( + r'^(python\.install)(\.)(\d+)(\.\w+)$', r'\1[\3]\4', key + ) + class BuildConfigBase: diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 394c9203d2f..0e469c93a2a 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1098,6 +1098,23 @@ def test_python_install_requirements_does_not_allow_null(self, tmpdir): build.validate() assert excinfo.value.key == 'python.install[0].requirements' + def test_python_install_requirements_error_msg(self, tmpdir): + build = self.get_build_config( + { + 'python': { + 'install': [{ + 'path': '.', + 'requirements': None, + }], + }, + }, + source_file=str(tmpdir.join('readthedocs.yml')), + ) + with raises(InvalidConfig) as excinfo: + build.validate() + + assert str(excinfo.value) == 'Invalid "python.install[0].requirements": expected string' + def test_python_install_requirements_does_not_allow_empty_string(self, tmpdir): build = self.get_build_config( { From a6c186b9eb35d85105d181ca40958b0e3aef1a77 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 25 Sep 2019 19:44:31 +0600 Subject: [PATCH 3/4] set regex only for msg --- readthedocs/config/config.py | 4 ++-- readthedocs/config/tests/test_config.py | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index c1c02561a8a..81be25c442c 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -110,11 +110,11 @@ class InvalidConfig(ConfigError): message_template = 'Invalid "{key}": {error}' def __init__(self, key, code, error_message, source_file=None): - self.key = self._get_display_key(key) + self.key = key self.code = code self.source_file = source_file message = self.message_template.format( - key=self.key, + key=self._get_display_key(key), code=code, error=error_message, ) diff --git a/readthedocs/config/tests/test_config.py b/readthedocs/config/tests/test_config.py index 0e469c93a2a..51befd21bef 100644 --- a/readthedocs/config/tests/test_config.py +++ b/readthedocs/config/tests/test_config.py @@ -1062,7 +1062,7 @@ def test_python_install_method_check_invalid(self, value, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install[0].method' + assert excinfo.value.key == 'python.install.0.method' def test_python_install_requirements_check_valid(self, tmpdir): apply_fs(tmpdir, {'requirements.txt': ''}) @@ -1096,7 +1096,7 @@ def test_python_install_requirements_does_not_allow_null(self, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install[0].requirements' + assert excinfo.value.key == 'python.install.0.requirements' def test_python_install_requirements_error_msg(self, tmpdir): build = self.get_build_config( @@ -1129,7 +1129,7 @@ def test_python_install_requirements_does_not_allow_empty_string(self, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install[0].requirements' + assert excinfo.value.key == 'python.install.0.requirements' def test_python_install_requirements_ignores_default(self, tmpdir): apply_fs(tmpdir, {'requirements.txt': ''}) @@ -1174,7 +1174,7 @@ def test_python_install_requirements_check_invalid_types(self, value, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install[0].requirements' + assert excinfo.value.key == 'python.install.0.requirements' def test_python_install_path_is_required(self, tmpdir): build = self.get_build_config( @@ -1324,7 +1324,7 @@ def test_python_install_extra_requirements_and_setuptools(self, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install[0].extra_requirements' + assert excinfo.value.key == 'python.install.0.extra_requirements' @pytest.mark.parametrize('value', [2, 'invalid', {}, '', None]) def test_python_install_extra_requirements_check_type(self, value, tmpdir): @@ -1342,7 +1342,7 @@ def test_python_install_extra_requirements_check_type(self, value, tmpdir): ) with raises(InvalidConfig) as excinfo: build.validate() - assert excinfo.value.key == 'python.install[0].extra_requirements' + assert excinfo.value.key == 'python.install.0.extra_requirements' def test_python_install_extra_requirements_allow_empty(self, tmpdir): build = self.get_build_config( @@ -1869,7 +1869,7 @@ def test_submodules_recursive_explicit_default(self): }] } }, - 'python.install[1].invalid' + 'python.install.1.invalid' ) ]) def test_strict_validation(self, value, key): From e8b3edfc5349f412bffcb315ad4980fb058c32d9 Mon Sep 17 00:00:00 2001 From: saadmk11 Date: Wed, 25 Sep 2019 21:43:00 +0600 Subject: [PATCH 4/4] covert staticmethod to instance method --- readthedocs/config/config.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/readthedocs/config/config.py b/readthedocs/config/config.py index 81be25c442c..b3d6b8aa875 100644 --- a/readthedocs/config/config.py +++ b/readthedocs/config/config.py @@ -114,18 +114,17 @@ def __init__(self, key, code, error_message, source_file=None): self.code = code self.source_file = source_file message = self.message_template.format( - key=self._get_display_key(key), + key=self._get_display_key(), code=code, error=error_message, ) super().__init__(message, code=code) - @staticmethod - def _get_display_key(key): + def _get_display_key(self): # Checks for patterns similar to `python.install.0.requirements` # if matched change to `python.install[0].requirements` using backreference. return re.sub( - r'^(python\.install)(\.)(\d+)(\.\w+)$', r'\1[\3]\4', key + r'^(python\.install)(\.)(\d+)(\.\w+)$', r'\1[\3]\4', self.key )