				# Copyright (C) 2020 The Software Heritage developers
				# See the AUTHORS file at the top-level directory of this distribution
				# License: GNU General Public License version 3, or any later version
				# See top-level LICENSE file for more information

				import os.path
				from pathlib import Path
				from unittest.mock import Mock

				from click.testing import CliRunner
				import pytest

				import swh.core.config as config
				import swh.scanner.cli as cli
				import swh.scanner.scanner as scanner

				douarddaUnsubmitted Not Done Inline Actions why are these fixtures? What aspect/feature of a fixture is used here? douardda: why are these fixtures? What aspect/feature of a fixture is used here?
				DATADIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), "data")
				TEST_CONFIG_PATH = os.path.join(DATADIR, "global.yml")

				ROOTPATH_GOOD = os.path.join(DATADIR, "")
				ROOTPATH_BAD = "~/not_expanduser-ed"
				douarddaUnsubmitted Not Done Inline Actions what??? what's this directory doing here? How can you be sure it exists? douardda: what??? what's this directory doing here? How can you be sure it exists?
				CONFPATH_GOOD = TEST_CONFIG_PATH
				CONFPATH_GOOD2 = TEST_CONFIG_PATH.replace("/swh/", "/swh/../swh/") # or a copy?
				CONFPATH_BAD = "~/not_expanduser-ed"

				GLOBAL_CONFIG_DATA = Path(TEST_CONFIG_PATH).read_text()
				douarddaUnsubmitted Not Done Inline Actions same, not a valid value. `~` may very well be a valid directory. If you want a directory that does not exists, you create a temp directory (that exists), than use a path that is a subdirectoy of this and you are sure does not exists (because you have full control of it). See e.g. the tmpdir fixture https://docs.pytest.org/en/stable/tmpdir.html douardda: same, not a valid value. `~` may very well be a valid directory. If you want a directory that…
				tenmaAuthorUnsubmitted Not Done Inline Actions In this case, as expand_user is never called, this directory cannot exist. Keep it and comment about this or not? tenma: In this case, as expand_user is never called, this directory cannot exist. Keep it and comment…


				@pytest.fixture(scope="function")
				def m_scanner(monkeypatch):
				"""Returns a mock swh.core.config object whose do_scan is noop"""
				douarddaUnsubmitted Not Done Inline Actions I don't get it how a unit test can expect to have this an existing file? Please use tmpdirs/tmpfiles for this kind of test. douardda: I don't get it how a unit test can expect to have this an existing file? Please use…
				# Customizable mock of scanner module
				# Fortunately, noop is the default behavior
				scanner_mock = Mock(scanner)
				monkeypatch.setattr("swh.scanner.scanner", scanner_mock)
				yield scanner_mock


				@pytest.fixture(scope="function")
				def m_config(monkeypatch):
				douarddaUnsubmitted Not Done Inline Actions not a big deal, but is this really a smoke test? Also if you test the `swh scanner --help` invocation, why not verify a few sanity checks are ok on the expected result (aka the help string)? douardda: not a big deal, but is this really a smoke test? Also if you test the `swh scanner --help`…
				"""Returns a mock swh.core.config object that return default config data"""
				import yaml

				douarddaUnsubmitted Not Done Inline Actions This is not a TODO, according the very nature of this diff, testing this is a must have. douardda: This is not a TODO, according the very nature of this diff, testing this is a must have.
				# Customizable mock of config module, with some methods not mocked
				# Whitelist approach, could be stricter with Mock.spec_set
				douarddaUnsubmitted Not Done Inline Actions note that making this comment a docstring makes pytest display it when the test fail (in verbose mode I believe). douardda: note that making this comment a docstring makes pytest display it when the test fail (in…
				config_mock = Mock(config)
				config_mock.config_exists = config.config_exists
				config_mock.read_raw_config = Mock(return_value=yaml.safe_load(GLOBAL_CONFIG_DATA))
				config_mock.merge_configs = config.merge_configs
				douarddaUnsubmitted Not Done Inline Actions Using these kind of helper functions (cli_run_[n]ok) as is (return a boolean) has a rather annoying drawbacks: on failure, the test only reports an "False is not True' kind of message, which is not so user friendly. It's better to implement theses as `assert_cli_run_ok` or silmilar and make them do the assertion (and use a better assertion message is possible). It's also prevent you from writing: assert call_function() which is not very meaningful for the fellow code reader (even if in this case the function name is rather explanatory). douardda: Using these kind of helper functions (cli_run_[n]ok) as is (return a boolean) has a rather…
				monkeypatch.setattr("swh.scanner.cli.config", config_mock)
				yield config_mock
				douarddaUnsubmitted Not Done Inline Actions it's unclear to me in what situation an execution raises an exception or prints error messages but does have a null exit code. Is this really happening ? If not, checking the exit code is enough. Otherwise, there is something else to fix :-) douardda: it's unclear to me in what situation an execution raises an exception or prints error messages…


				douarddaUnsubmitted Not Done Inline Actions oops douardda: oops
				@pytest.fixture(scope="function")
				def cli_runner():
				"""Return a CliRunner with default environment variable SWH_CONFIG_FILE unset"""
				return CliRunner(env={"SWH_CONFIG_FILE": ""})


				@pytest.fixture(scope="function")
				def good_default_config_path(monkeypatch):
				"""Patch default config path to a valid value"""
				assert os.path.exists(TEST_CONFIG_PATH)
				monkeypatch.setattr(cli, "DEFAULT_CONFIG_PATH", TEST_CONFIG_PATH)


				@pytest.fixture(scope="function")
				def bad_default_config_path(monkeypatch, tmpdir):
				"""Patch default config path to an invalid value"""
				default_config_path = os.path.join(tmpdir, "missing")
				monkeypatch.setattr(cli, "DEFAULT_CONFIG_PATH", default_config_path)


				def cli_run_ok(res):
				"""Return whether cli command ran successfully, given its result object"""
				return res.exit_code == 0 and res.exception is None and "Error:" not in res.stdout


				# TEST BEGIN

				# For nominal code paths, check that the right config file is loaded with a mock
				# mock scanner.scan to not run actual scan
				# TODO give ROOTPATH_GOOD an existing package within the temp directory
				douarddaUnsubmitted Not Done Inline Actions same, needs to be done in this diff. douardda: same, needs to be done in this diff.


				def test_smoke(cli_runner):
				"""Break if basic functionality breaks"""
				res = cli_runner.invoke(cli.scanner, ["scan", "-h"])
				assert cli_run_ok(res)


				def test_config_path_option_bad(cli_runner, bad_default_config_path):
				"""Test bad option no envvar bad default"""
				res = cli_runner.invoke(cli.scanner, ["-C", CONFPATH_BAD, "scan", ROOTPATH_GOOD])
				assert res.exit_code != 0 and res.exception.__class__ == FileNotFoundError


				def test_default_config_path(cli_runner, good_default_config_path, m_scanner, m_config):
				"""Test no option no envvar good default"""
				res = cli_runner.invoke(cli.scanner, ["scan", ROOTPATH_GOOD])
				assert cli_run_ok(res)
				m_config.read_raw_config.assert_called_with(
				m_config.config_basepath(TEST_CONFIG_PATH)
				)
				m_scanner.do_scan.assert_called_once()


				def test_root_no_config(cli_runner, bad_default_config_path, m_scanner, m_config):
				"""Test no config = no option no envvar bad default, good root"""
				res = cli_runner.invoke(cli.scanner, ["scan", ROOTPATH_GOOD])
				assert cli_run_ok(res)
				m_config.read_raw_config.assert_not_called()
				m_scanner.do_scan.assert_called_once()


				def test_root_bad(cli_runner, bad_default_config_path):
				"""Test no option no envvar bad default bad root"""
				res = cli_runner.invoke(cli.scanner, ["scan", ROOTPATH_BAD])
				assert res.exit_code == 2


				def test_config_path_envvar_good(
				cli_runner, bad_default_config_path, m_scanner, m_config
				):
				"""Test no option good envvar bad default good root"""
				cli_runner.env["SWH_CONFIG_FILE"] = CONFPATH_GOOD
				res = cli_runner.invoke(cli.scanner, ["scan", ROOTPATH_GOOD])
				assert cli_run_ok(res)
				m_config.read_raw_config.assert_called_with(m_config.config_basepath(CONFPATH_GOOD))
				m_scanner.do_scan.assert_called_once()


				def test_config_path_envvar_bad(cli_runner, bad_default_config_path):
				"""Test no option bad envvar bad default good root"""
				cli_runner.env["SWH_CONFIG_FILE"] = CONFPATH_BAD
				res = cli_runner.invoke(cli.scanner, ["scan", ROOTPATH_GOOD])
				assert res.exception.__class__ == FileNotFoundError


				def test_config_path_option_envvar(
				cli_runner, bad_default_config_path, m_scanner, m_config
				):
				"""Test good option good envvar bad default good root
				Check that option has precedence over envvar"""
				cli_runner.env["SWH_CONFIG_FILE"] = CONFPATH_GOOD2
				res = cli_runner.invoke(cli.scanner, ["-C", CONFPATH_GOOD, "scan", ROOTPATH_GOOD])
				assert cli_run_ok(res)
				m_config.read_raw_config.assert_called_with(m_config.config_basepath(CONFPATH_GOOD))
				m_scanner.do_scan.assert_called_once()


				def test_api_url_option(cli_runner, bad_default_config_path, m_scanner, m_config):
				"""Test no config good root good url"""
				api_url = cli.DEFAULT_CONFIG["web-api"]["url"]
				res = cli_runner.invoke(cli.scanner, ["scan", ROOTPATH_GOOD, "-u", api_url])
				assert cli_run_ok(res)
				m_config.read_raw_config.assert_not_called()
				m_scanner.do_scan.assert_called_once()

Enforce log level for the werkzeug logger.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 5996

swh/storage/cli.py

Enforce log level for the werkzeug logger.
ClosedPublic
Actions

Revision Contents
Changeset List