diff --git a/.flake8 b/.flake8 index 7163b1e..4af900b 100644 --- a/.flake8 +++ b/.flake8 @@ -68,6 +68,7 @@ ignore = WPS336, # WPS336 Found explicit string concatenation WPS338, # WPS338 Found incorrect order of methods in a class WPS360, # WPS360 Found an unnecessary use of a raw string + WPS402, # WPS402 Found `noqa` comments overuse: 12 WPS420, # WPS420 Found wrong keyword: pass WPS442, # WPS442 Found outer scope names shadowing: WPS600, # WPS600 Found subclassing a builtin: list @@ -82,6 +83,9 @@ per-file-ignores = transtool/report/error.py: WPS420, WPS604, transtool/report/warn.py: WPS420, WPS604, + # WPS437 Found protected attribute usage + transtool/report/group.py: WPS437, + # WPS230 Found too many public instance attributes transtool/config/config.py: WPS230, @@ -130,12 +134,12 @@ per-file-ignores = # WPS214 Found too many methods # WPS323 Found `%` string formatting # WPS432 Found magic number + # WPS437 Found protected attribute usage # WPS609 Found direct magic attribute usage: __abstractmethods__ - tests/*: S311, WPS323, WPS214, WPS432, WPS609, WPS118, + tests/*: S311, WPS323, WPS214, WPS432, WPS609, WPS118, WPS437, # WPS431 Found nested class: FakeArgs - # WPS437 Found protected attribute usage: _set_on_off_option - tests/report/test_config_builder.py: WPS431, WPS437, + tests/report/test_config_builder.py: WPS431, # WPS430 Found nested function: log_abort_side_effect # S311 Standard pseudo-random generators are not suitable for security/cryptographic purposes. diff --git a/docs/CHANGES.md b/docs/CHANGES.md index c497a84..f398363 100644 --- a/docs/CHANGES.md +++ b/docs/CHANGES.md @@ -6,6 +6,10 @@ # Changelog # +* v2.2.0 (2021-08-11) + * Added ability to specify checks to run with `--checks` option. + * Added more unit tests. + * v2.1.0 (2021-08-06) * Rebranded and renamed project to `trans-tool`. * Fixed config loader not handling Checks' sections properly. diff --git a/docs/config.md b/docs/config.md index 2737913..8cf1e19 100644 --- a/docs/config.md +++ b/docs/config.md @@ -10,7 +10,7 @@ * [« Documentation table of contents](README.md) * **Configuration** * [Config file](#config-file) - * [Syntax file](#syntax) + * [Syntax](#syntax) * [Structure](#structure) * [The [trans-tool] section](#trans-tool-section) @@ -27,7 +27,7 @@ any text editor. Please see commented [config.ini](../config.ini) for example co You should be using standard `UTF-8` encoding for your configuration file. -## Syntax ## +# Syntax # Configuration file is separated into sections. Each section starts with section header put in separate line between square brackets, followed by section related configuration items. Items are usually in `key = value` form. All keys are always lower-cased, and for @@ -99,6 +99,7 @@ The following elements are supported in `[trans-tool]` section | Key | CLI switch | Argument type | Default | Description | |-----------|---------------|---------------|---------|-------------| +| checks | `--checks` | List of strings | `[ Brackets, ... ]` | List of Checks IDs to be used for content validation. | | comment | `--comment` | String | `#` | TODO | | separator | `--separator` | String | `=` | TODO | | quiet | `--quiet` | Boolean | `false` | Quiet mode. If enabled, all but fatal errors are muted. | diff --git a/requirements3.txt b/requirements3.txt new file mode 100644 index 0000000..745be45 --- /dev/null +++ b/requirements3.txt @@ -0,0 +1,6 @@ +argparse +wemake-python-styleguide +pytest +wheel +setuptools +twine diff --git a/setup.py b/setup.py index baea613..696d0c5 100755 --- a/setup.py +++ b/setup.py @@ -23,6 +23,8 @@ with open('README.md', 'r') as fh: readme = fh.read() + readme.replace('![trans-tool logo](artwork/trans-tool-logo.png)', + 'https://raw.githubusercontent.com/MarcinOrlowski/trans-tool/master/artwork/trans-tool-logo.png', 1) setup( name = Const.APP_NAME, diff --git a/tests/checks/checks_test_case.py b/tests/checks/checks_test_case.py index 74a02c6..0247b42 100644 --- a/tests/checks/checks_test_case.py +++ b/tests/checks/checks_test_case.py @@ -104,7 +104,7 @@ def check_skipping_of_dangling_keys(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - trans_keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + trans_keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] # have less keys for reference file upper_bound = 10 diff --git a/tests/checks/test_dangling_keys.py b/tests/checks/test_dangling_keys.py index a4995d6..1256a7e 100644 --- a/tests/checks/test_dangling_keys.py +++ b/tests/checks/test_dangling_keys.py @@ -27,7 +27,7 @@ def test_translation_no_faults(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] ref_file = self.build_prepfile(keys) trans_file = self.build_prepfile(keys) self.check(trans_file, ref_file) @@ -36,7 +36,7 @@ def test_translation_with_faults(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - trans_keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + trans_keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] # have less keys for reference file upper_bound = 10 diff --git a/tests/checks/test_empty_translations.py b/tests/checks/test_empty_translations.py index 48e6a65..686c8bd 100644 --- a/tests/checks/test_empty_translations.py +++ b/tests/checks/test_empty_translations.py @@ -28,7 +28,7 @@ def test_no_faults(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] ref_file = self.build_prepfile(keys) trans_file = self.build_prepfile(keys) self.check(trans_file, ref_file) @@ -40,7 +40,7 @@ def test_if_both_are_empty(self) -> None: cnt_min = 10 cnt_max = 20 key_cnt = random.randint(cnt_min, cnt_max) - keys = [self.get_random_string('key_') for _ in range(key_cnt)] + keys = [self.get_random_string('key') for _ in range(key_cnt)] ref_file = self.build_prepfile(keys) trans_file = self.build_prepfile(keys) @@ -72,7 +72,7 @@ def test_translation_with_dangling_keys(self) -> None: cnt_min = 10 cnt_max = 20 key_cnt = random.randint(cnt_min, cnt_max) - keys = [self.get_random_string('key_') for _ in range(key_cnt)] + keys = [self.get_random_string('key') for _ in range(key_cnt)] ref_file = self.build_prepfile(keys) trans_file = self.build_prepfile(keys) @@ -100,7 +100,7 @@ def test_translation_with_faults(self) -> None: cnt_min = 10 cnt_max = 20 key_cnt = random.randint(cnt_min, cnt_max) - keys = [self.get_random_string('key_') for _ in range(key_cnt)] + keys = [self.get_random_string('key') for _ in range(key_cnt)] ref_file = self.build_prepfile(keys) trans_file = self.build_prepfile(keys) diff --git a/tests/checks/test_missing_translations.py b/tests/checks/test_missing_translations.py index 94e467c..3ec5c78 100644 --- a/tests/checks/test_missing_translations.py +++ b/tests/checks/test_missing_translations.py @@ -28,7 +28,7 @@ def test_no_faults(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] ref_file = self.build_prepfile(keys) trans_file = self.build_prepfile(keys) self.check(trans_file, ref_file) @@ -42,7 +42,7 @@ def test_translation_with_keys_in_comments(self) -> None: # Generate some keys for reference file. cnt_min = 10 cnt_max = 15 - ref_keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + ref_keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] # have less keys for translation file how_many_less = random.randint(1, cnt_min - 1) @@ -58,7 +58,7 @@ def test_translation_with_keys_in_comments(self) -> None: if random.randint(0, 1) == 0: comment = Comment.get_commented_out_key_comment(self.config, key) else: - val = self.get_random_string('translation_') + val = self.get_random_string('translation') comment = Comment.get_commented_out_key_comment(self.config, key, val) trans_file.append(comment) @@ -74,7 +74,7 @@ def test_translation_with_faults(self) -> None: # generate some keys for reference file cnt_min = 20 cnt_max = 40 - ref_keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + ref_keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] # have less keys for translation file how_many_less = random.randint(1, cnt_min - 1) diff --git a/tests/checks/test_punctuation.py b/tests/checks/test_punctuation.py index befbd5f..3dbe6b1 100644 --- a/tests/checks/test_punctuation.py +++ b/tests/checks/test_punctuation.py @@ -29,7 +29,7 @@ def test_no_faults(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] marks = self.checker.config['chars'] punct_idx = 0 @@ -48,7 +48,7 @@ def test_with_faults(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] marks = self.checker.config['chars'] punct_idx = 0 diff --git a/tests/checks/test_starts_with_the_same_case.py b/tests/checks/test_starts_with_the_same_case.py index 8c51d62..91904f8 100644 --- a/tests/checks/test_starts_with_the_same_case.py +++ b/tests/checks/test_starts_with_the_same_case.py @@ -29,7 +29,7 @@ def get_checker(self, config: Union[Dict, None] = None) -> Check: def test_no_faults(self) -> None: cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] ref_file = PropFile(self.config) trans_file = PropFile(self.config) @@ -43,7 +43,7 @@ def test_no_faults(self) -> None: def test_with_faults(self) -> None: cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] expected_faults = 0 @@ -68,7 +68,7 @@ def _do_scan_test(self, tests: List[Tuple[str, str]], exp_warnings = 0): for ref_value, trans_value in tests: ref_file = PropFile(self.config) trans_file = PropFile(self.config) - key = self.get_random_string('key_') + key = self.get_random_string('key') ref_file.append(Translation(key, ref_value)) trans_file.append(Translation(key, trans_value)) self.check(trans_file, ref_file, exp_warnings = exp_warnings) @@ -144,7 +144,7 @@ def test_skipping_of_entry_items(self) -> None: # generate some keys for translation file cnt_min = 20 cnt_max = 40 - keys = [self.get_random_string('key_') for _ in range(random.randint(cnt_min, cnt_max))] + keys = [self.get_random_string('key') for _ in range(random.randint(cnt_min, cnt_max))] ref_file = self.build_prepfile(keys, lower = True) trans_file = self.build_prepfile(keys, lower = True) diff --git a/tests/checks/test_substitutions.py b/tests/checks/test_substitutions.py new file mode 100644 index 0000000..bb95cfc --- /dev/null +++ b/tests/checks/test_substitutions.py @@ -0,0 +1,80 @@ +""" +# trans-tool +# The translation files checker and syncing tool. +# +# Copyright ©2021 Marcin Orlowski +# https://github.com/MarcinOrlowski/trans-tool/ +# +""" +from typing import Dict, Union, List + +from transtool.checks.substitutions import Substitutions +from transtool.decorators.overrides import overrides +from transtool.prop.file import PropFile +from transtool.prop.items import Comment, Translation +from tests.checks.checks_test_case import ChecksTestCase + + +class SubstitutionsTests(ChecksTestCase): + + @overrides(ChecksTestCase) + def get_checker(self, config: Union[Dict, None] = None) -> Substitutions: + return Substitutions(config) + + # ################################################################################################# + + def _get_valid_strings(self) -> List[str]: + return [ + 'Foo. Bar. All is fin5e!', + 'Foo!', + ] + + def _get_faulty_strings(self) -> List[str]: + return [ + 'Triple dots...', + 'Ough!!!', + ] + + def test_translation_no_faults(self) -> None: + for test in self._get_valid_strings(): + self.check_single_file(Translation('key', test)) + + def test_empty_translation(self) -> None: + self.check(PropFile(self.config)) + + # ################################################################################################# + + def test_comment_no_faults(self) -> None: + for test in self._get_valid_strings(): + self.check_single_file(Comment(test)) + + def test_comment_with_faults(self) -> None: + faults = self._get_faulty_strings() + + for fault in faults: + # We should see no issues if comment scanning is disabled. + self.checker.config['comments'] = False + self.check_single_file(Comment(fault)) + + # And some warnings when comment scanning in enabled. + self.checker.config['comments'] = True + self.check_single_file(Comment(fault), exp_warnings = 1) + + # ################################################################################################# + + def test_fail_with_error_flag(self) -> None: + """ + Ensures FLAG_FAIL_WITH_ERROR flag aborts scanning and returns error while + FLAG_DEFAULT yields warning. + """ + cfg = { + 'regexp': r'([\.]{3})', + 'replace': '…', + } + self.checker.config['map'] = [cfg] + + cfg['flag'] = Substitutions.FLAG_DEFAULT + self.check_single_file(Translation('key', 'Triple dots...'), exp_warnings = 1) + + cfg['flag'] = Substitutions.FLAG_FAIL_WITH_ERROR + self.check_single_file(Translation('key', 'Triple dots...'), exp_errors = 1) diff --git a/tests/checks/test_white_chars_before_linefeed.py b/tests/checks/test_white_chars_before_linefeed.py index fc3cddd..93f64a3 100644 --- a/tests/checks/test_white_chars_before_linefeed.py +++ b/tests/checks/test_white_chars_before_linefeed.py @@ -28,7 +28,7 @@ def test_translation_no_faults(self) -> None: self.check_single_file(Translation('key', r'This is\nall\nOK')) def test_translation_string_too_short(self) -> None: - # Checks behavior when the string is too short to fit space and litera, + # Checks behavior when the string is too short to fit space and literals, self.check_single_file(Translation('key', r' X')) self.check_single_file(Translation('key', r'\n')) diff --git a/tests/config/test_config.py b/tests/config/test_config.py new file mode 100644 index 0000000..8cdd993 --- /dev/null +++ b/tests/config/test_config.py @@ -0,0 +1,59 @@ +""" +# trans-tool +# The translation files checker and syncing tool. +# +# Copyright ©2021 Marcin Orlowski +# https://github.com/MarcinOrlowski/trans-tool/ +# +""" + +import random + +from tests.test_case import TestCase +from transtool.config.config import Config + + +class TestConfig(TestCase): + + def test_set_checker_config(self) -> None: + config = Config() + checker_id = self.get_random_string('checker_id') + checker_config = {} + max_items = 10 + for idx in range(random.randint(1, max_items)): + checker_config[self.get_random_string(f'key{idx}')] = self.get_random_string(f'val{idx}') + config.set_checker_config(checker_id, checker_config) + + self.assertIn(checker_id, config.checks) + self.assertEqual(len(checker_config), len(config.checks[checker_id])) + for key, val in checker_config.items(): + self.assertIn(key, config.checks[checker_id]) + self.assertEqual(val, config.checks[checker_id][key]) + + def test_set_checker_config_invalid_type(self) -> None: + config = Config() + checker_id = self.get_random_string('checker_id') + with self.assertRaises(TypeError): + # noinspection PyTypeChecker + config.set_checker_config(checker_id, False) # noqa: WPS425 + + def test_get_checker_config(self) -> None: + config = Config() + checker_id = self.get_random_string('checker_id') + checker_config = {} + max_items = 10 + for idx in range(random.randint(1, max_items)): + checker_config[self.get_random_string(f'key{idx}')] = self.get_random_string(f'val{idx}') + config.set_checker_config(checker_id, checker_config) + + read_config = config.get_checker_config(checker_id) + self.assertEqual(len(checker_config), len(read_config)) + for key, val in checker_config.items(): + self.assertIn(key, read_config) + self.assertEqual(val, read_config[key]) + + def test_get_checker_config_no_entry(self) -> None: + config = Config() + checker_id = self.get_random_string('checker_id') + with self.assertRaises(KeyError): + config.get_checker_config(checker_id) diff --git a/tests/config/test_config_builder.py b/tests/config/test_config_builder.py index c34cb08..a12c109 100644 --- a/tests/config/test_config_builder.py +++ b/tests/config/test_config_builder.py @@ -44,6 +44,9 @@ def __init__(self): self.__setattr__(option_name, False) self.__setattr__(f'no_{option_name}', False) + self.checkers = [] + self.checks = {} + class TestConfigBuilder(TestCase): @@ -53,12 +56,10 @@ def test_fake_args_matches_config(self) -> None: """ fake_args = FakeArgs() config = Config() - for key, val in config.__dict__.items(): + for key in config.__dict__: # Let's skip Checks' info. - if isinstance(val, dict): - continue - - self.assertIn(key, fake_args.__dict__, f'FakeArgs lacks "{key}" key."') + if key not in {'checks'}: # noqa: WPS525 + self.assertIn(key, fake_args.__dict__, f'FakeArgs lacks "{key}" key."') def test_fake_args_matches_argparse(self) -> None: """ @@ -70,7 +71,8 @@ def test_fake_args_matches_argparse(self) -> None: sys.argv[1:] = [] # noqa: WPS362 args = ConfigBuilder._parse_args() for key in fake_args.__dict__: - self.assertIn(key, args) + if key not in {'checks'}: # noqa: WPS525 + self.assertIn(key, args) # ################################################################################################# @@ -265,7 +267,7 @@ def test_add_file_suffix_missing_suffix(self) -> None: dests = copy.copy(srcs) # WHEN we process it ConfigBuilder._add_file_suffix(config, dests) - # THEN all file names should have file suffix appened. + # THEN all file names should have file suffix appended. for idx, src in enumerate(srcs): self.assertEqual(f'{str(src)}{config.file_suffix}', str(dests[idx])) @@ -378,6 +380,18 @@ def test_validate_args_invalid_comment_marker(self, log_e_mock: Mock) -> None: # ################################################################################################# + def test_get_checkers_from_args(self) -> None: + config = Config() + + # Pass no args for parsing (this is legit as we have config file that can provide what's needed). + sys.argv[1:] = [] # noqa: WPS362 + args = ConfigBuilder._parse_args() + for key in config.__dict__: + if key not in {'checks'}: # noqa: WPS525 + self.assertIn(key, args) + + # ################################################################################################# + def test_parse_args_returns_all_keys(self) -> None: """ Checks if argparse returned dict contains all the keys we expect to be present while building @@ -389,7 +403,8 @@ def test_parse_args_returns_all_keys(self) -> None: sys.argv[1:] = [] # noqa: WPS362 args = ConfigBuilder._parse_args() for key in config.__dict__: - self.assertIn(key, args) + if key not in {'checks'}: # noqa: WPS525 + self.assertIn(key, args) def test_parse_args_returns_no_more_keys(self) -> None: """ @@ -410,6 +425,7 @@ def test_parse_args_returns_no_more_keys(self) -> None: # FIXME: this should not be hardcoded here! del args['show_version'] del args['config_dump'] + del args['checkers'] for key in args: # Ensure key args returns is what is present in Config as well. @@ -423,7 +439,7 @@ def test_build(self) -> None: args = self._generate_fake_args(languages) config = Config() - file = self.get_random_string('file_') + file = self.get_random_string('file') config.files.append(file) # Pass no args for parsing (this is legit as we have config file that can provide what's needed). @@ -433,7 +449,7 @@ def test_build(self) -> None: ConfigBuilder.build(config) - # TODO: make comparision more detailed; add test for langs as well + # TODO: make comparison more detailed; add test for langs as well self.assertEqual(len(config.files), len(config.files)) for idx, def_file in enumerate(config.files): self.assertEqual(def_file, config.files[idx]) diff --git a/tests/prop/test_comment.py b/tests/prop/test_comment.py index 685a265..891b894 100644 --- a/tests/prop/test_comment.py +++ b/tests/prop/test_comment.py @@ -33,7 +33,7 @@ def test_without_marker(self) -> None: Checks if constructing Comment without valid marker in passed value would automatically add such marker. """ - val = self.get_random_string('no_maker_') + val = self.get_random_string('no_maker') # No valid comment marker comment = Comment(val) self.assertEqual(f'{Config.ALLOWED_COMMENT_MARKERS[0]} {val}', comment.to_string()) diff --git a/tests/prop/test_file.py b/tests/prop/test_file.py index 62045ac..f50de5c 100644 --- a/tests/prop/test_file.py +++ b/tests/prop/test_file.py @@ -11,6 +11,7 @@ from pathlib import Path from unittest.mock import Mock, mock_open, patch +from transtool.config.builder import ConfigBuilder from transtool.config.config import Config from transtool.prop.file import PropFile from transtool.prop.items import Blank, Comment, PropItem, Translation @@ -31,11 +32,11 @@ def _generate_propfile_with_content(self, config: Config) -> PropFile: new_items = 50 for new_item_cls in random.choices(item_types, item_weights, k = new_items): if issubclass(new_item_cls, Translation): - key = self.get_random_string('key_') - val = self.get_random_string('ref_val_') + key = self.get_random_string('key') + val = self.get_random_string('ref_val') propfile.append(Translation(key, val)) elif issubclass(new_item_cls, Comment): - propfile.append(Comment(self.get_random_string('comment_'))) + propfile.append(Comment(self.get_random_string('comment'))) elif issubclass(new_item_cls, Blank): propfile.append(Blank()) else: @@ -57,6 +58,18 @@ def test_append_wrong_arg_type(self) -> None: # noinspection PyTypeChecker propfile.append(obj) + def test_append_list_of_wrong_arg_type(self) -> None: + # GIVEN normal instance of PropFile + propfile = PropFile(Config()) + + # WHEN we try to append list of object of unsupported type + obj = ['INVALID'] + + # THEN Exception should be thrown. + with self.assertRaises(TypeError): + # noinspection PyTypeChecker + propfile.append(obj) + # ################################################################################################# def test_load_non_existing_file(self) -> None: @@ -224,18 +237,18 @@ def assertBlank(blank): self.assertIsNone(blank.value) comment1_marker = '#' - comment1_value = self.get_random_string('comment_') + comment1_value = self.get_random_string('comment') - key1 = self.get_random_string('key1_') + key1 = self.get_random_string('key1') sep1 = '=' - val1 = self.get_random_string('val1_') + val1 = self.get_random_string('val1') comment2_marker = '!' - comment2_value = self.get_random_string('comment_') + comment2_value = self.get_random_string('comment') - key2 = self.get_random_string('key2_') + key2 = self.get_random_string('key2') sep2 = '=' - val2 = self.get_random_string('val2_') + val2 = self.get_random_string('val2') fake_data_src = [ '', @@ -339,9 +352,10 @@ def test_save_no_target_file(self) -> None: # ################################################################################################# @patch('pathlib.Path.exists') - def test_is_valid_on_empty_files(self, path_exists_mock: Mock) -> None: - + def test_validate_on_empty_files(self, path_exists_mock: Mock) -> None: config = Config() + ConfigBuilder._setup_checkers(config) # noqa: WPS437 + fake_file = Path(f'/does/not/matter/{self.get_random_string()}') with patch('builtins.open', mock_open(read_data = '')): # Lie our fake file exists @@ -357,10 +371,29 @@ def test_is_valid_on_empty_files(self, path_exists_mock: Mock) -> None: propfile.report.dump() self.assertTrue(propfile.validate(ref_file)) + @patch('pathlib.Path.exists') + def test_validate_no_checkers_configured(self, path_exists_mock: Mock) -> None: + # GIVEN empty config without checkers set up + config = Config() + + # and GIVEN fake file + fake_file = Path(f'/does/not/matter/{self.get_random_string()}') + with patch('builtins.open', mock_open(read_data = '')): + # Lie our fake file exists + path_exists_mock.return_value = True + propfile = PropFile(config) + propfile.load(fake_file) + + # THEN we expect error raised + with self.assertRaises(RuntimeError): + # attempting to validate the file + propfile.validate(propfile) + # ################################################################################################# def test_update(self) -> None: config = Config() + ConfigBuilder._setup_checkers(config) # noqa: WPS437 # Generate reference file and its contents reference = self._generate_propfile_with_content(config) @@ -397,7 +430,7 @@ def test_update(self) -> None: for _ in range(max_items): translation.append([ Blank(), - Comment(self.get_random_string('comment_')), + Comment(self.get_random_string('comment')), ]) # THEN after update @@ -427,3 +460,20 @@ def test_update(self) -> None: continue self.fail(f'Unknown item type: {type(trans_item)}') + + def test_update_fail_on_unknown_type(self) -> None: + """ + Ensures unexpected and unsupported content will be caught and raise TypeError. + """ + config = Config() + + # GIVEN Translation and Reference objects + reference = PropFile(config) + trans = PropFile(config) + + # When reference contents contain invalid item + reference.items.append(False) + + # THEN attempt to update() should fail with error raised. + with self.assertRaises(TypeError): + trans.update(reference) diff --git a/tests/prop/test_translation.py b/tests/prop/test_translation.py index 563ce62..f93b06d 100644 --- a/tests/prop/test_translation.py +++ b/tests/prop/test_translation.py @@ -20,7 +20,7 @@ class SplitTest(TestCase): """ def __init__(self, fmt: str, mid_val_sep = True): - key = self.get_random_string('key_', length = 10) + key = self.get_random_string('key', length = 10) sep = random.choice(Config.ALLOWED_SEPARATORS) val_sep = random.choice(Config.ALLOWED_SEPARATORS) if mid_val_sep else '' val = self.get_random_string(length = 10) + val_sep + self.get_random_string(length = 10) @@ -142,7 +142,7 @@ def test_parse_translation_line_invalid_entries(self) -> None: def test_parse_translation_line_escaped_chars(self) -> None: key = r'this\=is\:\key' sep = random.choice(Config.ALLOWED_SEPARATORS) - val = self.get_random_string('value_') + val = self.get_random_string('value') line = f'{key}{sep}{val}' res = Translation.parse_translation_line(line) diff --git a/tests/report/test_report_group.py b/tests/report/test_report_group.py index aef8cdc..730bfa7 100644 --- a/tests/report/test_report_group.py +++ b/tests/report/test_report_group.py @@ -62,3 +62,23 @@ def test_error(self) -> None: self.assertIsInstance(item, Error) self.assertEqual(0, self.rg.warnings) self.assertEqual(1, self.rg.errors) + + def test_add_with_list(self) -> None: + """ + Ensures add() deals with all the cases properly. + :return: + """ + + reports = [ + Warn(None, msg = self.get_random_string()), + Error(None, msg = self.get_random_string()), + ] + nones = [ + None, + None, + ] + + rg = ReportGroup(self.get_random_string('label')) + rg.add(reports + nones) + # None's should be silently skipped + self.assertEqual(len(reports), rg.errors + rg.warnings) diff --git a/tests/test_case.py b/tests/test_case.py index bff05ab..66a49c2 100644 --- a/tests/test_case.py +++ b/tests/test_case.py @@ -17,7 +17,10 @@ class TestCase(unittest.TestCase): def get_random_string_list(self, count, str_prefix: str = '', str_length: int = 16) -> List[str]: return [self.get_random_string(str_prefix, str_length) for _ in range(count)] - def get_random_string(self, prefix: str = '', length: int = 16) -> str: + def get_random_string(self, prefix: str = '', length: int = 16, prefix_end = '_') -> str: + if prefix: + if prefix[-1] != prefix_end: + prefix += prefix_end for _ in range(length): single_char = chr(random.randint(ord('A'), ord('Z'))) if random.randint(0, 1): diff --git a/tests/test_utils.py b/tests/test_utils.py index 5d6eecd..bdb4072 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -82,7 +82,7 @@ def test_add_if_not_in_dict(self) -> None: # Try adding new elements for _ in range(count): - key = self.get_random_string('key_') + key = self.get_random_string('key') val = self.get_random_string() Utils.add_if_not_in_dict(src, key, val) Utils.add_if_not_in_dict(target, key, val) @@ -125,7 +125,7 @@ def test_remove_quotes_from_list(self) -> None: self.assertEqual(item, val) def test_remove_quotes_from_dict(self) -> None: - key = self.get_random_string('key_') + key = self.get_random_string('key') val_1 = self.get_random_string() val_2 = self.get_random_string() val_3 = self.get_random_string() diff --git a/transtool/checks/substitutions.py b/transtool/checks/substitutions.py index bdb0ba5..e8762fb 100644 --- a/transtool/checks/substitutions.py +++ b/transtool/checks/substitutions.py @@ -24,7 +24,8 @@ class Substitutions(Check): checks if all opened brackets are closed. """ - FAIL = -1 + FLAG_DEFAULT = 0 + FLAG_FAIL_WITH_ERROR = -1 def __init__(self, config: Union[Dict, None] = None): super().__init__(config) @@ -34,13 +35,15 @@ def _find_most_important_issue(self, idx: int, item: PropItem) -> Union[ReportIt warns = [] for config in self.config['map']: for match in re.finditer(config['regexp'], item.value): - if config['replace'] == self.FAIL: - return ReportGroup.build_error(f'{idx + 1}:{match.start()}', f'Invalid sequence "{match.group(1)}".', item.key) - else: - replacement = config['replace'] - warns.append( - ReportGroup.build_warn(f'{idx + 1}:{match.start()}', f'Sequence can be replaced with "{replacement}".', - item.key)) + at = f'{idx + 1}:{match.start()}' + + if 'flag' in config and config['flag'] == self.FLAG_FAIL_WITH_ERROR: + msg = f'Invalid sequence "{match.group(1)}".' + return ReportGroup.build_error(at, msg, item.key) + + replacement = config['replace'] + msg = f'Sequence at {at} can be replaced with "{replacement}".' + warns.append(ReportGroup.build_warn(at, msg, item.key)) return warns[0] if (isinstance(warns, list) and warns) else None @@ -50,18 +53,9 @@ def check(self, translation: 'PropFile', reference: 'PropFile' = None) -> Report self.need_valid_config() report = ReportGroup('Substitutions') - - if not translation.items: - return report - - for idx, item in enumerate(translation.items): - # Do not try to be clever and filter() data first, because line_number values will no longer be correct. - if self._shall_skip_item(item): - continue - - check_result = self._find_most_important_issue(idx, item) - if check_result is not None: - report.add(check_result) + if translation.items: + report.add([self._find_most_important_issue(idx, item) for idx, item in enumerate(translation.items) if + not self._shall_skip_item(item)]) return report @@ -71,23 +65,22 @@ def get_default_config(self) -> Dict: 'comments': False, # Keep matching elements at the same positions - 'map': [ + 'map': [ { - 'regexp': r'([\.]{3})', + 'regexp': r'([\.]{3})', 'replace': '…', - }, - { - 'regexp': r'([\.]{4,})', - 'replace': self.FAIL, - }, - - { - 'regexp': r'([\s]{2,})', + 'flag': self.FLAG_DEFAULT, + }, { + 'regexp': r'([\.]{4,})', + 'flag': self.FLAG_FAIL_WITH_ERROR, + }, { + 'regexp': r'([\s]{2,})', 'replace': ' ', - }, - { - 'regexp': r'([\!]{2,})', + 'flag': self.FLAG_DEFAULT, + }, { + 'regexp': r'([\!]{2,})', 'replace': '!', + 'flag': self.FLAG_DEFAULT, }, ], } diff --git a/transtool/config/builder.py b/transtool/config/builder.py index ab208e9..94c52c3 100644 --- a/transtool/config/builder.py +++ b/transtool/config/builder.py @@ -6,7 +6,6 @@ # https://github.com/MarcinOrlowski/trans-tool/ # """ - import argparse import re from pathlib import Path @@ -36,31 +35,30 @@ class ConfigBuilder(object): # List of options that can be either turned on or off. _on_off_pairs = [ - 'fatal', 'color', + 'fatal', + ] + + _default_checkers = [ + Brackets, + DanglingKeys, + EmptyTranslations, + FormattingValues, + KeyFormat, + MissingTranslations, + Punctuation, + QuotationMarks, + StartsWithTheSameCase, + Substitutions, + TrailingWhiteChars, + TypesettingQuotationMarks, + WhiteCharsBeforeLinefeed, ] @staticmethod def build(config_defaults: Config): - checkers = [ - Brackets, - DanglingKeys, - EmptyTranslations, - FormattingValues, - KeyFormat, - MissingTranslations, - Punctuation, - QuotationMarks, - StartsWithTheSameCase, - TrailingWhiteChars, - TypesettingQuotationMarks, - WhiteCharsBeforeLinefeed, - Substitutions, - ] - - for checker in checkers: - checker_id = checker.__name__ - config_defaults.checks[checker_id] = CheckerInfo(checker_id, checker, (checker()).get_default_config()) + # Let's populate default config with all the supported checkers first + ConfigBuilder._setup_checkers(config_defaults) # Handler CLI args so we can see if there's config file to load args = ConfigBuilder._parse_args() @@ -72,8 +70,18 @@ def build(config_defaults: Config): # override with command line arguments ConfigBuilder._set_from_args(config_defaults, args) + ConfigBuilder._get_checkers_from_args(config_defaults, args.checkers) + ConfigBuilder._validate_config(config_defaults) + @staticmethod + def _setup_checkers(config: Config, checkers_list: Union[List[str], None] = None) -> None: + if checkers_list is None: + checkers_list = ConfigBuilder._default_checkers + for checker in checkers_list: + checker_id = checker.__name__ + config.checks[checker_id] = CheckerInfo(checker_id, checker, (checker()).get_default_config()) + @staticmethod def _abort(msg: str) -> None: Log.e(msg) @@ -143,12 +151,29 @@ def _set_from_args(config: Config, args) -> None: ConfigBuilder._add_file_suffix(config, args.files) Utils.add_if_not_in_list(config.files, args.files) + @staticmethod + def _get_checkers_from_args(config: Config, args_checkers: Union[List[str], None]) -> None: + """ + If `--checks` argument list is provided, used checkers will be adjusted according to + values (Checker IDs) provided. + :param config: + :param args_checkers: + """ + all_checkers = {checker.__name__.lower(): checker for checker in ConfigBuilder._default_checkers} + if args_checkers: + checkers = [] + for checker_id in args_checkers: + if checker_id.lower() not in all_checkers: + ConfigBuilder._abort(f'Unknown checker ID "{checker_id}".') + Utils.add_if_not_in_list(checkers, checker_id) + ConfigBuilder._setup_checkers(config, checkers) + @staticmethod def _add_file_suffix(config: Config, files: Union[List[Path], None]) -> None: if files: suffix_len = len(config.file_suffix) for idx, file in enumerate(files): - # 'PosixPath' object is not subscriptable, so we cannot slice it. + # 'PosixPath' object cannot be sliced. path_str = str(file) if path_str[suffix_len * -1:] != config.file_suffix: files[idx] = Path(f'{path_str}{config.file_suffix}') @@ -185,7 +210,7 @@ def _parse_args() -> argparse: help = f'Default file name suffix. Default: "{Config.DEFAULT_FILE_SUFFIX}".') group = parser.add_argument_group('Checks controlling options') - group.add_argument('--checks', action = 'store', dest = 'checks', nargs = '+', metavar = 'CHECK_ID', + group.add_argument('--checks', action = 'store', dest = 'checkers', nargs = '+', metavar = 'CHECK_ID', help = 'List of checks ID to be executed. By default all available checks are run.') group.add_argument('-f', '--fatal', action = 'store_true', dest = 'fatal', diff --git a/transtool/config/config.py b/transtool/config/config.py index 5867787..2b03b73 100644 --- a/transtool/config/config.py +++ b/transtool/config/config.py @@ -60,6 +60,8 @@ def __init__(self): } def set_checker_config(self, checker_id: str, config: Dict) -> None: + if not isinstance(config, dict): + raise TypeError(f'Checker config must be a dictionary, {type(config)} given.') self.checks[checker_id] = config def get_checker_config(self, checker_id: str) -> Dict: @@ -67,7 +69,7 @@ def get_checker_config(self, checker_id: str) -> Dict: raise KeyError(f'No config for {checker_id} found.') return self.checks[checker_id] - def _dump(self, items: Dict): + def _dump_recursive(self, items: Dict): for key, val in items.items(): if isinstance(val, str): print(f'{key} = "{val}"') # noqa: WPS421 @@ -79,7 +81,7 @@ def _dump(self, items: Dict): if isinstance(val, dict): print() # noqa: WPS421 - self._dump(val) + self._dump_recursive(val) continue if isinstance(val, list): @@ -88,7 +90,7 @@ def _dump(self, items: Dict): if isinstance(val, CheckerInfo): print(f'[{val.id}]') # noqa: WPS421 - self._dump(val.config) + self._dump_recursive(val.config) print() # noqa: WPS421 continue @@ -100,4 +102,4 @@ def _dump(self, items: Dict): return def dump(self) -> None: - self._dump(self.__dict__['checks']) + self._dump_recursive(self.__dict__['checks']) diff --git a/transtool/config/reader.py b/transtool/config/reader.py index 2b89ad4..b30dcfe 100644 --- a/transtool/config/reader.py +++ b/transtool/config/reader.py @@ -69,6 +69,7 @@ def read(self, config: Config, config_file_name: Path) -> Config: self._merge_if_exists(self.parser, config.files, main_section, 'files') self._merge_if_exists(self.parser, config.languages, main_section, 'languages') + self._merge_if_exists(self.parser, config.checks, main_section, 'checks') if config.debug: for attr_name in dir(config): diff --git a/transtool/const.py b/transtool/const.py index 05b91ad..2146381 100644 --- a/transtool/const.py +++ b/transtool/const.py @@ -12,7 +12,7 @@ class Const(object): APP_NAME: str = 'trans-tool' - APP_VERSION: str = '2.1.0' + APP_VERSION: str = '2.2.0' APP_URL: str = 'https://github.com/MarcinOrlowski/trans-tool/' APP_DESCRIPTION: List[str] = [ diff --git a/transtool/main.py b/transtool/main.py index 15e1532..0a6a0a2 100644 --- a/transtool/main.py +++ b/transtool/main.py @@ -19,9 +19,6 @@ from .utils import Utils -# ################################################################################################# - - class TransTool(object): @staticmethod diff --git a/transtool/prop/file.py b/transtool/prop/file.py index 11109cb..d255b34 100644 --- a/transtool/prop/file.py +++ b/transtool/prop/file.py @@ -135,7 +135,7 @@ def update(self, reference: 'PropFile') -> None: else: tmp.append(Comment.get_commented_out_key_comment(self.config, item.key, item.value)) else: - raise RuntimeError(f'Unknown entry type: {type(item)} at position {idx + 1}') + raise TypeError(f'Unknown entry type: {type(item)} at position {idx + 1}') self._items = tmp.items self.keys = tmp.keys @@ -150,6 +150,9 @@ def validate(self, reference_file: 'PropFile') -> bool: :param reference_file: :return: True if file is valid, False if there were errors. """ + if not self.config.checks: + raise RuntimeError('Checks config element cannot be empty.') + for _, checker_info in self.config.checks.items(): checker = checker_info.callable(checker_info.config) # Each validator gets copy of the files, to prevent any potential destructive operation. diff --git a/transtool/report/group.py b/transtool/report/group.py index d2e0f17..48dcb90 100644 --- a/transtool/report/group.py +++ b/transtool/report/group.py @@ -6,8 +6,7 @@ # https://github.com/MarcinOrlowski/trans-tool/ # """ - -from typing import Union +from typing import Union, List from transtool.log import Log from transtool.report.items import Error, ReportItem, Warn @@ -42,29 +41,47 @@ def create(self, position: Union[str, int, None], msg: str, trans_key: Union[str def empty(self) -> bool: return (self.errors + self.warnings) == 0 - def add(self, item) -> None: - if not issubclass(type(item), ReportItem): - raise TypeError(f'Item must be subclass of ReportItem. "{type(item)}" given.') - self.append(item) - if isinstance(item, Warn): - self.warnings += 1 - else: - self.errors += 1 + def add(self, items: Union[ReportItem, List[ReportItem], None]) -> None: + if not items: + return + + if not isinstance(items, list): + items = [items] + + for item in items: + if item is None: + continue + if not issubclass(type(item), ReportItem): + raise TypeError(f'Item must be subclass of ReportItem. "{type(item)}" given.') + self.append(item) + if isinstance(item, Warn): + self.warnings += 1 + else: + self.errors += 1 + + @staticmethod + def _to_list(line: Union[str, int, None]) -> Union[List[str], None]: + """ + Accepts line variable (of various types) and converts to list of strings. + Raises TypeError is provided argument is of not supported type. + """ + if line: + if not isinstance(line, (str, int)): + raise TypeError(f'Unsupported argument type. "{type(line)}" given.') + if not isinstance(line, str): + line = str(line) + return line @staticmethod def build_warn(line: Union[str, int, None], msg: str, trans_key: Union[str, None] = None) -> Warn: - if line and not isinstance(line, str): - line = str(line) - return Warn(line, msg, trans_key) + return Warn(ReportGroup._to_list(line), msg, trans_key) def warn(self, line: Union[str, int, None], msg: str, trans_key: Union[str, None] = None) -> None: self.add(self.build_warn(line, msg, trans_key)) @staticmethod def build_error(line: Union[str, int, None], msg: str, trans_key: Union[str, None] = None) -> Error: - if isinstance(line, int): - line = str(line) - return Error(line, msg, trans_key) + return Error(ReportGroup._to_list(line), msg, trans_key) def error(self, line: Union[str, int, None], msg: str, trans_key: Union[str, None] = None) -> None: self.add(self.build_error(line, msg, trans_key))