From aba69e7c1f9070ceed81634ce846386c6a857090 Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Tue, 24 Oct 2023 14:34:25 +0200 Subject: [PATCH] Refactor and add tests Signed-off-by: Carmen Bianca BAKKER --- src/reuse/_licenses.py | 2 +- src/reuse/download.py | 34 +++++++++++++++++++++--------- src/reuse/init.py | 2 +- src/reuse/project.py | 3 +-- tests/test_download.py | 47 ++++++++++++++++++++++++++++++++++++++++++ tests/test_main.py | 36 ++++++++++++++++++++++++++++++-- 6 files changed, 108 insertions(+), 16 deletions(-) diff --git a/src/reuse/_licenses.py b/src/reuse/_licenses.py index 02ab89b12..9fea0fd10 100644 --- a/src/reuse/_licenses.py +++ b/src/reuse/_licenses.py @@ -11,8 +11,8 @@ import json import os -from typing import Dict, List, Tuple import re +from typing import Dict, List, Tuple _BASE_DIR = os.path.dirname(__file__) _RESOURCES_DIR = os.path.join(_BASE_DIR, "resources") diff --git a/src/reuse/download.py b/src/reuse/download.py index 2d7583a87..15891e5ac 100644 --- a/src/reuse/download.py +++ b/src/reuse/download.py @@ -14,7 +14,7 @@ from argparse import ArgumentParser, Namespace from gettext import gettext as _ from pathlib import Path -from typing import IO +from typing import IO, Optional from urllib.error import URLError from urllib.parse import urljoin @@ -64,7 +64,9 @@ def _path_to_license_file(spdx_identifier: str, root: StrPath) -> Path: def put_license_in_file( - spdx_identifier: str, destination: StrPath, out: IO[str] = None + spdx_identifier: str, + destination: StrPath, + source: Optional[StrPath] = None, ) -> None: """Download a license and put it in the destination file. @@ -73,6 +75,8 @@ def put_license_in_file( Args: spdx_identifier: SPDX License Identifier of the license. destination: Where to put the license. + source: Path to license file whose contents will be copied for + LicenseRef- files. Raises: URLError: if the license could not be downloaded. @@ -85,16 +89,15 @@ def put_license_in_file( if destination.exists(): raise FileExistsError(errno.EEXIST, "File exists", str(destination)) + # LicenseRef- license; don't download anything. if re.match(REF_RE, spdx_identifier): - if out is not None: - out.write(_("Enter path to custom license file")) - out.write("\n") - result = input().strip() - out.write("\n") - source = Path(result) + if source: + source = Path(source) if source.is_dir(): - source = source / "".join((spdx_identifier, ".txt")) + source = source / f"{spdx_identifier}.txt" shutil.copyfile(source, destination) + else: + destination.touch() else: text = download_license(spdx_identifier) with destination.open("w", encoding="utf-8") as fp: @@ -118,6 +121,15 @@ def add_arguments(parser: ArgumentParser) -> None: parser.add_argument( "--output", "-o", dest="file", action="store", type=PathType("w") ) + parser.add_argument( + "--source", + action="store", + type=PathType("r"), + help=_( + "source from which to copy custom LicenseRef- licenses, either" + " a directory that contains the file or the file itself" + ), + ) def run(args: Namespace, project: Project, out: IO[str] = sys.stdout) -> int: @@ -171,7 +183,9 @@ def _successfully_downloaded(destination: StrPath) -> None: else: destination = _path_to_license_file(lic, project.root) try: - put_license_in_file(lic, destination=destination) + put_license_in_file( + lic, destination=destination, source=args.source + ) except URLError: _could_not_download(lic) return_code = 1 diff --git a/src/reuse/init.py b/src/reuse/init.py index 476e1c7c6..dc7da8417 100644 --- a/src/reuse/init.py +++ b/src/reuse/init.py @@ -123,7 +123,7 @@ def run( try: out.write(_("Retrieving {}").format(lic)) out.write("\n") - put_license_in_file(lic, destination=destination, out=out) + put_license_in_file(lic, destination=destination) # TODO: exceptions except FileExistsError: out.write(_("{} already exists").format(destination)) diff --git a/src/reuse/project.py b/src/reuse/project.py index 7a7e21ed8..c99e72417 100644 --- a/src/reuse/project.py +++ b/src/reuse/project.py @@ -11,9 +11,8 @@ import glob import logging import os -import warnings import re - +import warnings from gettext import gettext as _ from pathlib import Path from typing import Dict, Iterator, List, Optional, Union, cast diff --git a/tests/test_download.py b/tests/test_download.py index 44e9dc691..36bc18f8f 100644 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -104,3 +104,50 @@ def test_put_empty_dir(empty_directory, monkeypatch): assert (empty_directory / "LICENSES").exists() assert (empty_directory / "LICENSES/0BSD.txt").read_text() == "hello\n" + + +def test_put_custom_without_source(fake_repository): + """When 'downloading' a LicenseRef license without source, create an empty + file. + """ + put_license_in_file("LicenseRef-hello", "LICENSES/LicenseRef-hello.txt") + + assert (fake_repository / "LICENSES/LicenseRef-hello.txt").exists() + assert (fake_repository / "LICENSES/LicenseRef-hello.txt").read_text() == "" + + +def test_put_custom_with_source(fake_repository): + """When 'downloading' a LicenseRef license with source file, copy the source + text. + """ + (fake_repository / "foo.txt").write_text("foo") + + put_license_in_file( + "LicenseRef-hello", + "LICENSES/LicenseRef-hello.txt", + source=fake_repository / "foo.txt", + ) + + assert (fake_repository / "LICENSES/LicenseRef-hello.txt").exists() + assert ( + fake_repository / "LICENSES/LicenseRef-hello.txt" + ).read_text() == "foo" + + +def test_put_custom_with_source_dir(fake_repository): + """When 'downloading' a LicenseRef license with source directory, copy the + source text from a matching file in the directory. + """ + (fake_repository / "lics").mkdir() + (fake_repository / "lics/LicenseRef-hello.txt").write_text("foo") + + put_license_in_file( + "LicenseRef-hello", + "LICENSES/LicenseRef-hello.txt", + source=fake_repository / "lics", + ) + + assert (fake_repository / "LICENSES/LicenseRef-hello.txt").exists() + assert ( + fake_repository / "LICENSES/LicenseRef-hello.txt" + ).read_text() == "foo" diff --git a/tests/test_main.py b/tests/test_main.py index 342b4b6cf..6f4879201 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -385,7 +385,7 @@ def test_download(fake_repository, stringio, mock_put_license_in_file): assert result == 0 mock_put_license_in_file.assert_called_with( - "0BSD", Path("LICENSES/0BSD.txt").resolve() + "0BSD", Path("LICENSES/0BSD.txt").resolve(), source=None ) @@ -434,7 +434,9 @@ def test_download_custom_output( result = main(["download", "-o", "foo", "0BSD"], out=stringio) assert result == 0 - mock_put_license_in_file.assert_called_with("0BSD", destination=Path("foo")) + mock_put_license_in_file.assert_called_with( + "0BSD", destination=Path("foo"), source=None + ) def test_download_custom_output_too_many( @@ -449,6 +451,36 @@ def test_download_custom_output_too_many( ) +def test_download_licenseref_no_source(empty_directory, stringio): + """Downloading a LicenseRef license creates an empty file.""" + main(["download", "LicenseRef-hello"]) + assert (empty_directory / "LICENSES/LicenseRef-hello.txt").read_text() == "" + + +def test_download_licenseref_source_file(empty_directory, stringio): + """Downloading a LicenseRef license with a source file copies that file's + contents. + """ + (empty_directory / "foo.txt").write_text("foo") + main(["download", "--source", "foo.txt", "LicenseRef-hello"]) + assert ( + empty_directory / "LICENSES/LicenseRef-hello.txt" + ).read_text() == "foo" + + +def test_download_licenseref_source_dir(empty_directory, stringio): + """Downloading a LicenseRef license with a source dir copies the text from + the corresponding file in the directory. + """ + (empty_directory / "lics").mkdir() + (empty_directory / "lics/LicenseRef-hello.txt").write_text("foo") + + main(["download", "--source", "lics", "LicenseRef-hello"]) + assert ( + empty_directory / "LICENSES/LicenseRef-hello.txt" + ).read_text() == "foo" + + def test_supported_licenses(stringio): """Invoke the supported-licenses command and check whether the result contains at least one license in the expected format.