From bd588eb9a4b2890da6f9db928f302e2e98a7b9ce Mon Sep 17 00:00:00 2001 From: Wojtek Kosior Date: Tue, 10 May 2022 16:47:47 +0200 Subject: add missing english translations and verify message texts of raised exceptions in tests --- conftest.py | 33 ++++ src/hydrilla/builder/build.py | 5 +- src/hydrilla/builder/local_apt.py | 14 +- .../locales/en_US/LC_MESSAGES/hydrilla-messages.po | 90 ++++++++--- tests/test_build.py | 45 ++++-- tests/test_local_apt.py | 169 ++++++++++++++++----- 6 files changed, 269 insertions(+), 87 deletions(-) diff --git a/conftest.py b/conftest.py index 141cba5..65d13d5 100644 --- a/conftest.py +++ b/conftest.py @@ -8,6 +8,7 @@ import sys from pathlib import Path import pytest +import pkgutil here = Path(__file__).resolve().parent sys.path.insert(0, str(here / 'src')) @@ -34,3 +35,35 @@ def mock_subprocess_run(monkeypatch, request): run = mocked_run monkeypatch.setattr(where, 'subprocess', MockedSubprocess) + +@pytest.fixture(autouse=True) +def no_gettext(monkeypatch, request): + """ + Make gettext return all strings untranslated unless we request otherwise. + """ + if request.node.get_closest_marker('enable_gettext'): + return + + import hydrilla + modules_to_process = [hydrilla] + + def add_child_modules(parent): + """ + Recursuvely collect all modules descending from 'parent' into an array. + """ + try: + load_paths = parent.__path__ + except AttributeError: + return + + for module_info in pkgutil.iter_modules(load_paths): + if module_info.name != '__main__': + __import__(f'{parent.__name__}.{module_info.name}') + modules_to_process.append(getattr(parent, module_info.name)) + add_child_modules(getattr(parent, module_info.name)) + + add_child_modules(hydrilla) + + for module in modules_to_process: + if hasattr(module, '_'): + monkeypatch.setattr(module, '_', lambda message: message) diff --git a/src/hydrilla/builder/build.py b/src/hydrilla/builder/build.py index ce4935c..feecdfe 100644 --- a/src/hydrilla/builder/build.py +++ b/src/hydrilla/builder/build.py @@ -83,10 +83,11 @@ def generate_spdx_report(root: Path) -> bytes: try: cp = subprocess.run(command, capture_output=True, text=True) except FileNotFoundError: - raise ReuseError(_('couldnt_execute_reuse_is_it_installed')) + msg = _('couldnt_execute_{}_is_it_installed').format('reuse') + raise ReuseError(msg) if cp.returncode != 0: - msg = _('reuse_command_{}_failed').format(' '.join(command)) + msg = _('command_{}_failed').format(' '.join(command)) raise ReuseError(msg, cp) return cp.stdout.encode() diff --git a/src/hydrilla/builder/local_apt.py b/src/hydrilla/builder/local_apt.py index 8382af8..4c93a4d 100644 --- a/src/hydrilla/builder/local_apt.py +++ b/src/hydrilla/builder/local_apt.py @@ -114,10 +114,11 @@ class Apt: try: cp = run(command, **kwargs) except FileNotFoundError: - raise AptError(_('couldnt_execute_apt_get_is_it_installed')) + msg = _('couldnt_execute_{}_is_it_installed').format('apt-get') + raise AptError(msg) if cp.returncode != 0: - msg = _('apt_get_command_{}_failed').format(' '.join(command)) + msg = _('command_{}_failed').format(' '.join(command)) raise AptError(msg, cp) return cp @@ -185,7 +186,7 @@ def apt_keyring(keys: [str]) -> bytes: try: from gnupg import GPG except ModuleNotFoundError: - raise GpgError(_('couldnt_import_gnupg_is_it_installed')) + raise GpgError(_('couldnt_import_{}_is_it_installed').format('gnupg')) gpg = GPG(keyring=str(cache_dir() / 'master_keyring.gpg')) for key in keys: @@ -193,7 +194,7 @@ def apt_keyring(keys: [str]) -> bytes: continue if gpg.recv_keys(default_keyserver, key).imported == 0: - raise GpgError(_('gpg_couldnt_recv_key')) + raise GpgError(_('gpg_couldnt_recv_key_{}').format(key)) return gpg.export_keys(keys, armor=False, minimal=True) @@ -404,10 +405,11 @@ def piggybacked_system(piggyback_def: dict, foreign_packages: Optional[Path]) \ try: cp = run(command) except FileNotFoundError: - raise AptError(_('couldnt_execute_dpkg_deb_is_it_installed')) + msg = _('couldnt_execute_{}_is_it_installed'.format('dpkg-deb')) + raise AptError(msg) if cp.returncode != 0: - msg = _('dpkg_deb_command_{}_failed').format(' '.join(command)) + msg = _('command_{}_failed').format(' '.join(command)) raise AptError(msg, cp) docs_dir = root / 'usr' / 'share' / 'doc' diff --git a/src/hydrilla/builder/locales/en_US/LC_MESSAGES/hydrilla-messages.po b/src/hydrilla/builder/locales/en_US/LC_MESSAGES/hydrilla-messages.po index e3ab525..a6c6e83 100644 --- a/src/hydrilla/builder/locales/en_US/LC_MESSAGES/hydrilla-messages.po +++ b/src/hydrilla/builder/locales/en_US/LC_MESSAGES/hydrilla-messages.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: hydrilla.builder 0.1.dev16+g4e46d7f.d20220211\n" "Report-Msgid-Bugs-To: koszko@koszko.org\n" -"POT-Creation-Date: 2022-04-19 13:51+0200\n" +"POT-Creation-Date: 2022-05-10 16:47+0200\n" "PO-Revision-Date: 2022-02-12 00:00+0000\n" "Last-Translator: Wojtek Kosior \n" "Language: en_US\n" @@ -18,45 +18,63 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Generated-By: Babel 2.8.0\n" -#: src/hydrilla/builder/build.py:118 -msgid "couldnt_import_reuse_is_it_installed" -msgstr "" -"Could not import 'reuse'. Is the tool installed and visible to this " -"Python instance?" +#: src/hydrilla/builder/build.py:86 src/hydrilla/builder/local_apt.py:117 +#: src/hydrilla/builder/local_apt.py:408 +msgid "couldnt_execute_{}_is_it_installed" +msgstr "Could not execute '{}'. Is the tool installed and reachable via PATH?" + +#: src/hydrilla/builder/build.py:90 src/hydrilla/builder/local_apt.py:121 +#: src/hydrilla/builder/local_apt.py:412 +msgid "command_{}_failed" +msgstr "The following command finished execution with a non-zero exit status: {}" -#: src/hydrilla/builder/build.py:123 -msgid "spdx_report_from_reuse_incompliant" -msgstr "Attempt to generate an SPDX report for a REUSE-incompliant package." +#: src/hydrilla/builder/build.py:198 +msgid "path_contains_double_dot_{}" +msgstr "" +"Attempt to load '{}' which includes a forbidden parent reference ('..') " +"in the path." -#: src/hydrilla/builder/build.py:207 +#: src/hydrilla/builder/build.py:205 msgid "loading_{}_outside_package_dir" msgstr "Attempt to load '{}' which lies outside package source directory." -#: src/hydrilla/builder/build.py:211 +#: src/hydrilla/builder/build.py:209 msgid "loading_reserved_index_json" msgstr "Attempt to load 'index.json' which is a reserved filename." -#: src/hydrilla/builder/build.py:329 +#: src/hydrilla/builder/build.py:350 msgid "report_spdx_not_in_copyright_list" msgstr "" "Told to generate 'report.spdx' but 'report.spdx' is not listed among " "copyright files. Refusing to proceed." -#: src/hydrilla/builder/build.py:402 +#: src/hydrilla/builder/build.py:421 +msgid "build_package_from_srcdir_to_dstdir" +msgstr "" +"Build Hydrilla package from `scrdir` and write the resulting files under " +"`dstdir`." + +#: src/hydrilla/builder/build.py:423 msgid "source_directory_to_build_from" msgstr "Source directory to build from." -#: src/hydrilla/builder/build.py:404 +#: src/hydrilla/builder/build.py:425 msgid "path_instead_of_index_json" msgstr "" "Path to file to be processed instead of index.json (if not absolute, " "resolved relative to srcdir)." -#: src/hydrilla/builder/build.py:406 +#: src/hydrilla/builder/build.py:427 +msgid "path_instead_for_piggyback_files" +msgstr "" +"Path to a non-standard directory with foreign packages' archive files to " +"use." + +#: src/hydrilla/builder/build.py:429 msgid "built_package_files_destination" msgstr "Destination directory to write built package files to." -#: src/hydrilla/builder/build.py:408 +#: src/hydrilla/builder/build.py:431 #, python-format msgid "%(prog)s_%(version)s_license" msgstr "" @@ -67,15 +85,45 @@ msgstr "" "This is free software: you are free to change and redistribute it.\n" "There is NO WARRANTY, to the extent permitted by law." -#: src/hydrilla/builder/build.py:409 +#: src/hydrilla/builder/build.py:432 msgid "version_printing" msgstr "Print version information and exit." -#: src/hydrilla/builder/build.py:415 -msgid "build_package_from_srcdir_to_dstdir" +#: src/hydrilla/builder/common_errors.py:62 +msgid "STDOUT_OUTPUT_heading" +msgstr "## Command's standard output ##" + +#: src/hydrilla/builder/common_errors.py:65 +msgid "STDERR_OUTPUT_heading" +msgstr "## Command's standard error output ##" + +#: src/hydrilla/builder/local_apt.py:146 +msgid "distro_{}_unknown" +msgstr "Attempt to use an unknown software distribution '{}'." + +#: src/hydrilla/builder/local_apt.py:189 +msgid "couldnt_import_{}_is_it_installed" msgstr "" -"Build Hydrilla package from `scrdir` and write the resulting files under " -"`dstdir`." +"Could not import '{}'. Is the module installed and visible to this Python" +" instance?" + +#: src/hydrilla/builder/local_apt.py:197 +msgid "gpg_couldnt_recv_key_{}" +msgstr "Could not import PGP key '{}'." + +#: src/hydrilla/builder/local_apt.py:314 +msgid "apt_install_output_not_understood" +msgstr "The output of an 'apt-get install' command was not understood." + +#: src/hydrilla/builder/local_apt.py:342 +msgid "apt_download_gave_bad_filename_{}" +msgstr "The 'apt-get download' command produced a file with unexpected name '{}'." + +#: src/hydrilla/builder/piggybacking.py:102 +msgid "loading_{}_outside_piggybacked_dir" +msgstr "" +"Attempt to load '{}' which lies outside piggybacked packages files root " +"directory." #: src/hydrilla/util/_util.py:79 msgid "bad_comment" diff --git a/tests/test_build.py b/tests/test_build.py index a30cff4..77b1898 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -582,49 +582,64 @@ def sample_source_error_missing_file(monkeypatch, sample_source): Modify index.json to expect missing report.spdx file and cause an error. """ monkeypatch.delitem(index_obj, 'reuse_generate_spdx_report') - return FileNotFoundError + return FileNotFoundError, @error_maker def sample_source_error_index_schema(monkeypatch, sample_source): """Modify index.json to be incompliant with the schema.""" monkeypatch.delitem(index_obj, 'definitions') - return ValidationError + return ValidationError, @error_maker def sample_source_error_bad_comment(monkeypatch, sample_source): """Modify index.json to have an invalid '/' in it.""" - return json.JSONDecodeError, json.dumps(index_obj) + '/something\n' + return json.JSONDecodeError, '^bad_comment: .*', \ + json.dumps(index_obj) + '/something\n' @error_maker def sample_source_error_bad_json(monkeypatch, sample_source): """Modify index.json to not be valid json even after comment stripping.""" - return json.JSONDecodeError, json.dumps(index_obj) + '???/\n' + return json.JSONDecodeError, '', json.dumps(index_obj) + '???\n' @error_maker def sample_source_error_missing_reuse(monkeypatch, sample_source): """Cause mocked reuse process invocation to fail with FileNotFoundError.""" (sample_source / 'mock_reuse_missing').touch() - return build.ReuseError + return build.ReuseError, '^couldnt_execute_reuse_is_it_installed$' @error_maker def sample_source_error_missing_license(monkeypatch, sample_source): """Remove a file to make package REUSE-incompliant.""" (sample_source / 'README.txt.license').unlink() - return build.ReuseError + + error_regex = """^\ +command_reuse --root \\S+ lint_failed + +STDOUT_OUTPUT_heading + +dummy lint output + +STDERR_OUTPUT_heading + +some error output\ +$\ +""" + + return build.ReuseError, error_regex @error_maker def sample_source_error_file_outside(monkeypatch, sample_source): """Make index.json illegally reference a file outside srcdir.""" new_list = [*index_obj['copyright'], {'file': '../abc'}] monkeypatch.setitem(index_obj, 'copyright', new_list) - return FileReferenceError + return FileReferenceError, '^path_contains_double_dot_\\.\\./abc$' @error_maker def sample_source_error_reference_itself(monkeypatch, sample_source): """Make index.json illegally reference index.json.""" new_list = [*index_obj['copyright'], {'file': 'index.json'}] monkeypatch.setitem(index_obj, 'copyright', new_list) - return FileReferenceError + return FileReferenceError, '^loading_reserved_index_json$' @error_maker def sample_source_error_report_excluded(monkeypatch, sample_source): @@ -635,7 +650,7 @@ def sample_source_error_report_excluded(monkeypatch, sample_source): new_list = [file_ref for file_ref in index_obj['copyright'] if file_ref['file'] != 'report.spdx'] monkeypatch.setitem(index_obj, 'copyright', new_list) - return FileReferenceError + return FileReferenceError, '^report_spdx_not_in_copyright_list$' @pytest.fixture(params=error_makers) def sample_source_make_errors(request, monkeypatch, sample_source): @@ -644,10 +659,8 @@ def sample_source_make_errors(request, monkeypatch, sample_source): broken versions. Return an error type that should be raised when running test build. """ - index_text = None - error_type = request.param(monkeypatch, sample_source) - if type(error_type) is tuple: - error_type, index_text = error_type + error_type, error_regex, index_text = \ + [*request.param(monkeypatch, sample_source), '', ''][0:3] index_text = index_text or json.dumps(index_obj) @@ -655,13 +668,13 @@ def sample_source_make_errors(request, monkeypatch, sample_source): monkeypatch.setitem(src_files, 'index.json', index_text.encode()) - return error_type + return error_type, error_regex @pytest.mark.subprocess_run(build, run_reuse) @pytest.mark.usefixtures('mock_subprocess_run') def test_build_error(tmpdir, sample_source, sample_source_make_errors): """Try building the sample source package and verify generated errors.""" - error_type = sample_source_make_errors + error_type, error_regex = sample_source_make_errors dstdir = Path(tmpdir) / 'dstdir' tmpdir = Path(tmpdir) / 'example' @@ -669,6 +682,6 @@ def test_build_error(tmpdir, sample_source, sample_source_make_errors): dstdir.mkdir(exist_ok=True) tmpdir.mkdir(exist_ok=True) - with pytest.raises(error_type): + with pytest.raises(error_type, match=error_regex): build.Build(sample_source, Path('index.json'))\ .write_package_files(dstdir) diff --git a/tests/test_local_apt.py b/tests/test_local_apt.py index 4f3a831..1ff52cc 100644 --- a/tests/test_local_apt.py +++ b/tests/test_local_apt.py @@ -290,14 +290,11 @@ def test_local_apt_missing(mock_cache_dir): """ sources_list = local_apt.SourcesList(['deb-src sth', 'deb sth']) - with pytest.raises(local_apt.AptError) as excinfo: + with pytest.raises(local_apt.AptError, + match='^couldnt_execute_apt-get_is_it_installed$'): with local_apt.local_apt(sources_list, local_apt.default_keys) as apt: pass - assert len(excinfo.value.args) == 1 - assert isinstance(excinfo.value.args[0], str) - assert '\n' not in excinfo.value.args[0] - @pytest.mark.subprocess_run(local_apt, make_run_apt_get(update_code=1)) @pytest.mark.usefixtures('mock_subprocess_run', 'mock_gnupg_import') def test_local_apt_update_fail(mock_cache_dir): @@ -307,14 +304,22 @@ def test_local_apt_update_fail(mock_cache_dir): """ sources_list = local_apt.SourcesList(['deb-src sth', 'deb sth']) - with pytest.raises(local_apt.AptError) as excinfo: - with local_apt.local_apt(sources_list, local_apt.default_keys) as apt: - pass + error_regex = """^\ +command_apt-get -c \\S+ update_failed + +STDOUT_OUTPUT_heading + +some output - assert len(excinfo.value.args) == 1 +STDERR_OUTPUT_heading - assert re.match(r'.*\n\n.*\n\nsome output\n\n.*\n\nsome error output', - excinfo.value.args[0]) +some error output\ +$\ +""" + + with pytest.raises(local_apt.AptError, match=error_regex): + with local_apt.local_apt(sources_list, local_apt.default_keys) as apt: + pass @pytest.mark.subprocess_run(local_apt, make_run_apt_get()) @pytest.mark.usefixtures('mock_subprocess_run', 'mock_gnupg_import') @@ -359,17 +364,24 @@ def test_local_apt_install_fail(mock_cache_dir): destination = mock_cache_dir / 'destination' destination.mkdir() - with pytest.raises(local_apt.AptError) as excinfo: + error_regex = f"""^\ +command_apt-get -c \\S+ install --yes --just-print libjs-mathjax_failed + +STDOUT_OUTPUT_heading + +{re.escape(sample_install_stdout)} + +STDERR_OUTPUT_heading + +some error output\ +$\ +""" + + with pytest.raises(local_apt.AptError, match=error_regex): local_apt.download_apt_packages(sources_list, local_apt.default_keys, ['libjs-mathjax'], destination, with_deps=True) - assert len(excinfo.value.args) == 1 - - assert re.match(r'^.*\n\n.*\n\n', excinfo.value.args[0]) - assert re.search(r'\n\nsome error output$', excinfo.value.args[0]) - assert sample_install_stdout in excinfo.value.args[0] - assert [*destination.iterdir()] == [] @pytest.mark.subprocess_run(local_apt, make_run_apt_get(download_code=1)) @@ -383,14 +395,73 @@ def test_local_apt_download_fail(mock_cache_dir): destination = mock_cache_dir / 'destination' destination.mkdir() - with pytest.raises(local_apt.AptError) as excinfo: + error_regex = """^\ +command_apt-get -c \\S+ download libjs-mathjax_failed + +STDOUT_OUTPUT_heading + +some output + +STDERR_OUTPUT_heading + +some error output\ +$\ +""" + + with pytest.raises(local_apt.AptError, match=error_regex): local_apt.download_apt_packages(sources_list, local_apt.default_keys, ['libjs-mathjax'], destination) - assert len(excinfo.value.args) == 1 + assert [*destination.iterdir()] == [] - assert re.match(r'.*\n\n.*\n\nsome output\n\n.*\n\nsome error output', - excinfo.value.args[0]) +@pytest.fixture +def mock_bad_deb_file(monkeypatch, mock_subprocess_run): + """ + Make mocked 'apt-get download' command produce an incorrectly-named file. + """ + old_run = local_apt.subprocess.run + + def twice_mocked_run(command, **kwargs): + """ + Create an evil file if needed; then act just like the run() function + that got replaced by this one. + """ + if 'download' in command: + destination = Path(kwargs.get('cwd') or Path.cwd()) + (destination / 'arbitrary-name').write_text('anything') + + return old_run(command, **kwargs) + + monkeypatch.setattr(local_apt.subprocess, 'run', twice_mocked_run) + +@pytest.mark.subprocess_run(local_apt, make_run_apt_get()) +@pytest.mark.usefixtures('mock_subprocess_run', 'mock_gnupg_import', + 'mock_bad_deb_file') +def test_local_apt_download_bad_filename(mock_cache_dir): + """ + Verify that the download_apt_packages() function raises a proper error when + 'apt-get download' command produces an incorrectly-named file. + """ + sources_list = local_apt.SourcesList([], 'nabia') + destination = mock_cache_dir / 'destination' + destination.mkdir() + + error_regex = """^\ +apt_download_gave_bad_filename_arbitrary-name + +STDOUT_OUTPUT_heading + +some output + +STDERR_OUTPUT_heading + +some error output\ +$\ +""" + + with pytest.raises(local_apt.AptError, match=error_regex): + local_apt.download_apt_packages(sources_list, local_apt.default_keys, + ['libjs-mathjax'], destination) assert [*destination.iterdir()] == [] @@ -405,14 +476,22 @@ def test_local_apt_source_fail(mock_cache_dir): destination = mock_cache_dir / 'destination' destination.mkdir() - with pytest.raises(local_apt.AptError) as excinfo: - local_apt.download_apt_packages(sources_list, local_apt.default_keys, - ['libjs-mathjax'], destination) + error_regex = """^\ +command_apt-get -c \\S* source --download-only \\S+_failed + +STDOUT_OUTPUT_heading - assert len(excinfo.value.args) == 1 +some output - assert re.match(r'.*\n\n.*\n\nsome output\n\n.*\n\nsome error output', - excinfo.value.args[0]) +STDERR_OUTPUT_heading + +some error output\ +$\ +""" + + with pytest.raises(local_apt.AptError, match=error_regex): + local_apt.download_apt_packages(sources_list, local_apt.default_keys, + ['libjs-mathjax'], destination) assert [*destination.iterdir()] == [] @@ -421,7 +500,7 @@ def test_sources_list(): list = local_apt.SourcesList([], 'nabia') assert list.identity() == 'nabia' - with pytest.raises(local_apt.DistroError): + with pytest.raises(local_apt.DistroError, match='^distro_nabiał_unknown$'): local_apt.SourcesList([], 'nabiał') list = local_apt.SourcesList(['deb sth', 'deb-src sth'], 'nabia') @@ -557,7 +636,9 @@ def test_piggybacked_system_download(params): assert piggybacked.resolve_file(PurePosixPath('a/b/c')) == None assert piggybacked.resolve_file(PurePosixPath('')) == None - with pytest.raises(FileReferenceError): + output_text = 'loading_.apt-root/a/../../../b_outside_piggybacked_dir' + with pytest.raises(FileReferenceError, + match=f'^{re.escape(output_text)}$'): piggybacked.resolve_file(PurePosixPath('.apt-root/a/../../../b')) root = piggybacked.resolve_file(PurePosixPath('.apt-root/dummy')).parent @@ -615,7 +696,8 @@ def test_piggybacked_system_missing(): Verify that the piggybacked_system() function raises a proper error when 'dpkg-deb' is missing. """ - with pytest.raises(local_apt.AptError) as excinfo: + with pytest.raises(local_apt.AptError, + match='^couldnt_execute_dpkg-deb_is_it_installed$'): with local_apt.piggybacked_system({ 'system': 'apt', 'distribution': 'nabia', @@ -624,11 +706,6 @@ def test_piggybacked_system_missing(): }, None) as piggybacked: pass - assert len(excinfo.value.args) == 1 - - assert '\n' not in excinfo.value.args[0] - - @pytest.mark.subprocess_run(local_apt, lambda c, **kw: run_dpkg_deb(c, 1, **kw)) @pytest.mark.usefixtures('mock_download_packages', 'mock_subprocess_run') def test_piggybacked_system_fail(): @@ -636,7 +713,20 @@ def test_piggybacked_system_fail(): Verify that the piggybacked_system() function raises a proper error when 'dpkg-deb -x' command returns non-0. """ - with pytest.raises(local_apt.AptError) as excinfo: + error_regex = """^\ +command_dpkg-deb -x \\S+\\.deb \\S+_failed + +STDOUT_OUTPUT_heading + +some output + +STDERR_OUTPUT_heading + +some error output\ +$\ +""" + + with pytest.raises(local_apt.AptError, match=error_regex): with local_apt.piggybacked_system({ 'system': 'apt', 'distribution': 'nabia', @@ -644,8 +734,3 @@ def test_piggybacked_system_fail(): 'dependencies': False }, None) as piggybacked: pass - - assert len(excinfo.value.args) == 1 - - assert re.match(r'.*\n\n.*\n\nsome output\n\n.*\n\nsome error output', - excinfo.value.args[0]) -- cgit v1.2.3