From b30f593a352a0cb444af06abb5f4e513f02d4214 Mon Sep 17 00:00:00 2001 From: Yannick de Jong Date: Thu, 3 Sep 2026 11:17:36 +0200 Subject: [PATCH 1/2] Add CLI tests --- tests/cli/cli_helpers.py | 34 + tests/cli/conftest.py | 159 ++++ tests/cli/test_cli_alias_command.py | 166 ++-- tests/cli/test_cli_config_command.py | 116 ++- tests/cli/test_cli_display.py | 337 ++++++++ tests/cli/test_cli_manifest_command.py | 105 +-- .../cli/test_cli_optional_remote_argument.py | 105 +++ tests/cli/test_cli_provenance_command.py | 70 +- tests/cli/test_cli_remote_api_client.py | 374 +++++++++ tests/cli/test_cli_remote_command.py | 730 +++++++++++------- tests/cli/test_cli_remote_config_command.py | 152 ++++ tests/cli/test_cli_root.py | 133 ++++ tests/cli/test_cli_simulation_command.py | 579 ++++++++++++-- tests/cli/test_cli_validators.py | 28 + tests/cli/test_remote_api_helpers.py | 42 + tests/cli/utils.py | 51 -- 16 files changed, 2594 insertions(+), 587 deletions(-) create mode 100644 tests/cli/cli_helpers.py create mode 100644 tests/cli/conftest.py create mode 100644 tests/cli/test_cli_display.py create mode 100644 tests/cli/test_cli_optional_remote_argument.py create mode 100644 tests/cli/test_cli_remote_api_client.py create mode 100644 tests/cli/test_cli_remote_config_command.py create mode 100644 tests/cli/test_cli_root.py create mode 100644 tests/cli/test_cli_validators.py create mode 100644 tests/cli/test_remote_api_helpers.py delete mode 100644 tests/cli/utils.py diff --git a/tests/cli/cli_helpers.py b/tests/cli/cli_helpers.py new file mode 100644 index 00000000..18a69a92 --- /dev/null +++ b/tests/cli/cli_helpers.py @@ -0,0 +1,34 @@ +"""Helpers shared by the CLI tests. + +Kept out of ``conftest.py`` because pytest imports every ``conftest`` under +the same module name, so importing from it directly picks up whichever one +was loaded first. +""" + +from typing import Optional +from unittest import mock + + +def make_simulation( + alias: str, + uuid: str = "0123456789abcdef0123456789abcdef", + datetime: str = "2000-01-01 00:00:00", + status: str = "not validated", + meta: Optional[dict] = None, +) -> mock.Mock: + """Build a stand-in for a :class:`~simdb.database.models.Simulation`. + + Only the attributes the CLI display code touches are set; ``find_meta`` + answers from ``meta`` the same way the real model does (a list of objects + with a ``value``, empty when the name is unknown). + """ + simulation = mock.Mock() + simulation.alias = alias + simulation.uuid = uuid + simulation.datetime = datetime + simulation.status = status + meta = meta or {} + simulation.find_meta.side_effect = lambda name: ( + [mock.Mock(value=meta[name])] if name in meta else [] + ) + return simulation diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py new file mode 100644 index 00000000..ff2c5342 --- /dev/null +++ b/tests/cli/conftest.py @@ -0,0 +1,159 @@ +"""Shared fixtures for the ``simdb`` command line interface tests. + +Every test here drives the CLI through :class:`click.testing.CliRunner`, so the +fixtures below take care of the two things that would otherwise leak between +tests and into the developer's machine: the configuration that the CLI reads at +startup, and the handshake :class:`~simdb.cli.remote_api.RemoteAPI` performs +against a remote when it is constructed. +""" + +import os +from pathlib import Path +from types import SimpleNamespace +from unittest import mock + +import pytest +from click.testing import CliRunner, Result + +from simdb.cli.remote_api import RemoteAPI +from simdb.cli.simdb import cli + +REMOTE_NAME = "test" +REMOTE_URL = "http://0.0.0.0:5000/" +REMOTE_TOKEN = "123ABC" + +SERVER_ENDPOINTS = ["v1", "v1.1", "v1.1.1", "v1.2", "v1.3"] +"""API versions the fake remote advertises.""" + + +@pytest.fixture(autouse=True) +def isolated_config_environment(tmp_path, monkeypatch): + """Point the CLI at throw-away site and user configuration files. + + :class:`~simdb.config.config.Config` reads ``simdb.cfg`` from the platform + config directories unless ``SIMDB_SITE_CONFIG_PATH``/ + ``SIMDB_USER_CONFIG_PATH`` say otherwise. Without this fixture the outcome of + a test depends on whether the machine running it happens to have a real + SimDB configuration, which is exactly the kind of difference that makes a + suite pass locally and fail in CI. + """ + for variable in [name for name in os.environ if name.startswith("SIMDB_")]: + monkeypatch.delenv(variable) + monkeypatch.setenv("SIMDB_SITE_CONFIG_PATH", str(tmp_path / "site-simdb.cfg")) + monkeypatch.setenv("SIMDB_USER_CONFIG_PATH", str(tmp_path / "user-simdb.cfg")) + + +@pytest.fixture +def config_file(tmp_path) -> Path: + """A configuration file declaring a single, default, token-authenticated remote.""" + config_path = tmp_path / "simdb.cfg" + config_path.write_text( + f'[remote "{REMOTE_NAME}"]\n' + f"url = {REMOTE_URL}\n" + "default = True\n" + f"token = {REMOTE_TOKEN}\n" + "\n" + "[db]\n" + # Keep any command that reaches the real database away from the local + # one in the user's data directory. + f"file = {tmp_path / 'sim.db'}\n" + ) + return config_path + + +@pytest.fixture +def runner() -> CliRunner: + return CliRunner() + + +@pytest.fixture +def invoke(runner, config_file): + """Invoke the ``simdb`` CLI against the throw-away :func:`config_file`. + + ``invoke("simulation", "list")`` runs ``simdb --config-file=... simulation + list``. Any keyword argument is forwarded to + :meth:`click.testing.CliRunner.invoke`, so ``input=`` can be used to answer + prompts. + """ + + def _invoke(*args: str, **kwargs) -> Result: + return runner.invoke(cli, [f"--config-file={config_file}", *args], **kwargs) + + return _invoke + + +@pytest.fixture +def remote_handshake(): + """Stub the requests :class:`RemoteAPI` makes while it is being constructed. + + ``RemoteAPI.__init__`` asks the remote for its authentication scheme, its + endpoints, and its server version before any command specific request is + made. Tests that only care about the command itself get all three stubbed + here, and can still assert on them through the returned namespace:: + + def test_something(invoke, remote_handshake): + ... + assert remote_handshake.get_endpoints.called + """ + with mock.patch.object( + RemoteAPI, "get_server_authentication", return_value="None" + ) as get_server_authentication, mock.patch.object( + RemoteAPI, "get_endpoints", return_value=list(SERVER_ENDPOINTS) + ) as get_endpoints, mock.patch.object( + RemoteAPI, "get_server_version", return_value="0.11" + ) as get_server_version: + yield SimpleNamespace( + get_server_authentication=get_server_authentication, + get_endpoints=get_endpoints, + get_server_version=get_server_version, + ) + + +@pytest.fixture +def local_db(): + """Replace the local database with a mock in every module that looks it up. + + ``get_local_db`` is imported into each command module, so patching a single + import site silently leaves the other commands talking to the real database + in the user's data directory. + """ + db = mock.Mock() + with mock.patch( + "simdb.cli.commands.alias.get_local_db", return_value=db + ), mock.patch("simdb.cli.commands.simulation.get_local_db", return_value=db): + yield db + + +@pytest.fixture +def data_file(tmp_path) -> Path: + """A small file a manifest can reference as an input or output.""" + path = tmp_path / "data.txt" + path.write_text("simulation data\n") + return path + + +@pytest.fixture +def manifest_file(tmp_path, data_file) -> Path: + """A minimal, valid manifest referencing only local files.""" + manifest_path = tmp_path / "manifest.yaml" + manifest_path.write_text( + f"""\ +manifest_version: 2 +alias: simulation-alias + +inputs: + - uri: file://{data_file} + +outputs: + - uri: file://{data_file} + +metadata: +- values: + workflow: + name: Workflow Name + git: ssh://git@git.iter.org/wf/workflow.git + branch: master + commit: 079e84d5ae8a0eec6dcf3819c98f3c05f48e952f +""" + ) + return manifest_path diff --git a/tests/cli/test_cli_alias_command.py b/tests/cli/test_cli_alias_command.py index c173dfd6..a85a6efd 100644 --- a/tests/cli/test_cli_alias_command.py +++ b/tests/cli/test_cli_alias_command.py @@ -1,80 +1,110 @@ -from unittest import mock +"""Tests for ``simdb alias``.""" -from click.testing import CliRunner -from utils import config_test_file +from unittest import mock -from simdb.cli.simdb import cli +import pytest +from cli_helpers import make_simulation LOCAL_ALIASES = ["hello", "world", "foo-123"] REMOTE_ALIASES = ["foo#1", "bar", "barfoo", "123foo", "barbaz"] -def _generate_mock_data(get_local_db, remote_list_simulations): - simulations = [] - - for alias in REMOTE_ALIASES: - sim = mock.Mock() - sim.alias = alias - simulations.append(sim) - remote_list_simulations.return_value = simulations - simulations = [] - - for alias in LOCAL_ALIASES: - sim = mock.Mock() - sim.alias = alias - simulations.append(sim) - get_local_db.return_value.list_simulations.return_value = simulations - - -@mock.patch("simdb.cli.commands.alias.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -@mock.patch("simdb.cli.remote_api.RemoteAPI.__init__") -def test_alias_search_command(init, remote_list_simulations, get_local_db): - init.return_value = None - _generate_mock_data(get_local_db, remote_list_simulations) - - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "alias", "search", "foo"] - ) - assert result.exception is None - expected_sims = ["foo#1", "barfoo", "123foo", "foo-123"] - assert "\n".join(expected_sims) in result.output - - -@mock.patch("simdb.cli.commands.alias.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -@mock.patch("simdb.cli.remote_api.RemoteAPI.has_url") -@mock.patch("simdb.cli.remote_api.RemoteAPI.__init__") -def test_alias_list_command(init, has_url, remote_list_simulations, get_local_db): - init.return_value = None - has_url.return_value = True - _generate_mock_data(get_local_db, remote_list_simulations) - - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "alias", "list"]) - assert result.exception is None +@pytest.fixture +def aliases(local_db, remote_handshake): + """A local database and a remote, each holding a known set of aliases.""" + local_db.list_simulations.return_value = [ + make_simulation(alias) for alias in LOCAL_ALIASES + ] + with mock.patch( + "simdb.cli.remote_api.RemoteAPI.list_simulations", + return_value=[make_simulation(alias) for alias in REMOTE_ALIASES], + ), mock.patch("simdb.cli.remote_api.RemoteAPI.has_url", return_value=True): + yield + + +def test_search_returns_local_and_remote_matches(invoke, aliases): + result = invoke("alias", "search", "foo") + + assert result.exit_code == 0 + assert "\n".join(["foo#1", "barfoo", "123foo", "foo-123"]) in result.output + + +def test_search_without_matches_prints_nothing(invoke, aliases): + result = invoke("alias", "search", "nothing-matches-this") + + assert result.exit_code == 0 + for alias in LOCAL_ALIASES + REMOTE_ALIASES: + assert alias not in result.output + + +def test_list_shows_the_local_aliases(invoke, aliases): + result = invoke("alias", "list") + + assert result.exit_code == 0 assert "\n ".join(LOCAL_ALIASES) in result.output -@mock.patch("simdb.cli.commands.alias.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -@mock.patch("simdb.cli.remote_api.RemoteAPI.has_url") -@mock.patch("simdb.cli.remote_api.RemoteAPI.__init__") -def test_alias_list_command_with_remote_name( - init, has_url, remote_list_simulations, get_local_db -): - init.return_value = None - has_url.return_value = True - _generate_mock_data(get_local_db, remote_list_simulations) - - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "alias", "test", "list"] - ) - assert result.exception is None +def test_list_with_a_remote_name_shows_both_sides(invoke, aliases): + result = invoke("alias", "test", "list") + + assert result.exit_code == 0 assert "\n ".join(REMOTE_ALIASES) in result.output assert "\n ".join(LOCAL_ALIASES) in result.output + + +def test_list_can_skip_the_remote(invoke, aliases): + result = invoke("alias", "list", "--local") + + assert result.exit_code == 0 + assert "Remote:" not in result.output + assert "\n ".join(LOCAL_ALIASES) in result.output + + +def test_list_explains_a_remote_without_a_url(invoke, local_db, remote_handshake): + local_db.list_simulations.return_value = [] + with mock.patch("simdb.cli.remote_api.RemoteAPI.has_url", return_value=False): + result = invoke("alias", "list") + + assert result.exit_code == 0 + assert "The Remote Server has not been specified" in result.output + + +def test_make_unique_returns_an_unused_alias_unchanged(invoke, aliases): + result = invoke("alias", "make-unique", "brand-new") + + assert result.exit_code == 0 + assert result.output.strip() == "brand-new" + + +def test_make_unique_appends_a_counter_to_a_taken_alias(invoke, aliases): + result = invoke("alias", "make-unique", "bar") + + assert result.exit_code == 0 + assert result.output.strip() == "bar-1" + + +def test_make_unique_replaces_reserved_characters(invoke, aliases): + result = invoke("alias", "make-unique", "a#b/c(d)e=f,g*h%i") + + assert result.exit_code == 0 + assert result.output.strip() == "a_b_c_d_e_f_g_h_i" + + +def test_make_unique_keeps_counting_past_a_taken_suffix(invoke, aliases, local_db): + local_db.list_simulations.return_value = [ + make_simulation("bar"), + make_simulation("bar-1"), + ] + + result = invoke("alias", "make-unique", "bar") + + assert result.exit_code == 0 + assert result.output.strip() == "bar-2" + + +def test_the_group_shows_help_when_no_subcommand_is_given(invoke): + result = invoke("alias") + + assert result.exit_code == 0 + assert "Query remote and local aliases." in result.output + assert "make-unique" in result.output diff --git a/tests/cli/test_cli_config_command.py b/tests/cli/test_cli_config_command.py index 1d278d9b..853c1b10 100644 --- a/tests/cli/test_cli_config_command.py +++ b/tests/cli/test_cli_config_command.py @@ -1,36 +1,96 @@ +"""Tests for ``simdb config``.""" + from unittest import mock -from click.testing import CliRunner -from utils import config_test_file +import pytest from simdb.cli.simdb import cli -@mock.patch("simdb.config.config.Config.get_option") -def test_config_get(get_option): - config_file = config_test_file() - get_option.return_value = "bar" - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "config", "get", "foo"] - ) - assert result.exception is None +def test_get_prints_the_option_value(invoke): + with mock.patch( + "simdb.config.config.Config.get_option", return_value="bar" + ) as get_option: + result = invoke("config", "get", "foo") + + assert result.exit_code == 0 assert "bar" in result.output - (args, kwargs) = get_option.call_args - assert args == ("foo",) - assert kwargs == {} - - -@mock.patch("simdb.config.config.Config.save") -@mock.patch("simdb.config.config.Config.set_option") -def test_config_set(set_option, save): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "config", "set", "foo", "bar"] - ) - assert result.exception is None - (args, kwargs) = set_option.call_args - assert args == ("foo", "bar") - assert kwargs == {} + # Config.load also reads options, so only the final call is the command's. + assert get_option.call_args.args == ("foo",) + + +def test_get_reads_from_the_loaded_config_file(invoke): + result = invoke("config", "get", "remote.test.url") + + assert result.exit_code == 0 + assert "http://0.0.0.0:5000/" in result.output + + +def test_get_fails_for_an_unknown_option(invoke): + result = invoke("config", "get", "no.such.option") + + assert result.exit_code != 0 + + +def test_set_stores_the_option_and_saves(invoke): + with mock.patch("simdb.config.config.Config.save") as save, mock.patch( + "simdb.config.config.Config.set_option" + ) as set_option: + result = invoke("config", "set", "foo", "bar") + + assert result.exit_code == 0 + # Config.load sets options for the SIMDB_* environment variables first. + assert set_option.call_args.args == ("foo", "bar") + assert save.called + + +def test_delete_removes_the_option_and_saves(invoke): + with mock.patch("simdb.config.config.Config.save") as save, mock.patch( + "simdb.config.config.Config.delete_option" + ) as delete_option: + result = invoke("config", "delete", "foo") + + assert result.exit_code == 0 + assert "Success." in result.output + delete_option.assert_called_once_with("foo") assert save.called + + +def test_list_shows_the_configured_options(invoke): + result = invoke("config", "list") + + assert result.exit_code == 0 + assert "remote.test.url: http://0.0.0.0:5000/" in result.output + + +def test_list_masks_remote_tokens(invoke): + """A token in the configuration must never be echoed back in full.""" + result = invoke("config", "list") + + assert result.exit_code == 0 + assert "remote.test.token: ********" in result.output + assert "123ABC" not in result.output + + +def test_path_prints_the_config_file_that_was_loaded(invoke, tmp_path): + """``--config-file`` replaces the user configuration rather than adding to it.""" + result = invoke("config", "path") + + assert result.exit_code == 0 + assert str(tmp_path / "simdb.cfg") in result.output + + +def test_path_falls_back_to_the_user_configuration(runner, tmp_path): + """Without ``--config-file`` the location comes from SIMDB_USER_CONFIG_PATH.""" + result = runner.invoke(cli, ["config", "path"]) + + assert result.exit_code == 0 + assert str(tmp_path / "user-simdb.cfg") in result.output + + +@pytest.mark.parametrize("subcommand", ["get", "set", "delete"]) +def test_missing_arguments_are_reported(invoke, subcommand): + result = invoke("config", subcommand) + + assert result.exit_code == 2 + assert "Missing argument" in result.output diff --git a/tests/cli/test_cli_display.py b/tests/cli/test_cli_display.py new file mode 100644 index 00000000..f88bf18b --- /dev/null +++ b/tests/cli/test_cli_display.py @@ -0,0 +1,337 @@ +"""Tests for the console output helpers in ``simdb.cli.commands.utils``. + +These are the functions that turn simulations and IDS quantities into what the +user actually sees. They are pure enough to call directly, so they are tested +here without going through a command. +""" + +import pytest +from cli_helpers import make_simulation +from rich.console import Console + +from simdb.cli.commands import utils +from simdb.cli.commands.utils import ( + is_numeric_1d, + print_quantity, + print_simulations, + print_trace, + show_quantity_textual_plot, +) +from simdb.remote.models import QuantityData, SimulationTraceData + + +@pytest.fixture(autouse=True) +def fixed_width_console(monkeypatch): + """Give the rich output a stable width so assertions do not depend on the + terminal the tests happen to run in.""" + monkeypatch.setattr( + utils, "_RICH_CONSOLE", Console(width=120, legacy_windows=False) + ) + + +def quantity(data, name="grid/rho_tor_norm", units="-") -> QuantityData: + return QuantityData(name=name, units=units, data=data) + + +# --------------------------------------------------------------------------- +# is_numeric_1d +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("data", "expected"), + [ + ([1, 2, 3], True), + ([1.0, 2.5], True), + ([], False), + ([[1, 2], [3, 4]], False), + (["a", "b"], False), + # bool is an int subclass, but plotting booleans is not what the caller + # means by "numeric". + ([True, False], False), + (1.0, False), + (None, False), + ], +) +def test_is_numeric_1d(data, expected): + assert is_numeric_1d(data) is expected + + +# --------------------------------------------------------------------------- +# print_quantity +# --------------------------------------------------------------------------- + + +def test_print_quantity_renders_a_scalar(capsys): + print_quantity(quantity(1.23456789, name="time", units="s")) + + output = capsys.readouterr().out + assert "1.23457" in output + assert "scalar" in output + + +def test_print_quantity_renders_a_one_dimensional_array_with_stats(capsys): + print_quantity(quantity([0.0, 0.5, 1.0])) + + output = capsys.readouterr().out + assert "shape (3,)" in output + for column in ("n", "min", "max", "mean", "std", "median"): + assert column in output + assert "0.5" in output + + +def test_print_quantity_truncates_long_rows(capsys): + print_quantity(quantity(list(range(100)))) + + output = capsys.readouterr().out + assert "..." in output + assert "shape (100,)" in output + # Head and tail are kept, the middle is not. + assert "0 1 2" in output + assert "97 98 99" in output + assert "50" not in output + + +def test_print_quantity_renders_a_two_dimensional_array(capsys): + print_quantity(quantity([[1, 2], [3, 4]])) + + output = capsys.readouterr().out + assert "shape (2, 2)" in output + + +def test_print_quantity_truncates_tall_two_dimensional_arrays(capsys): + print_quantity(quantity([[row, row] for row in range(20)])) + + output = capsys.readouterr().out + assert "shape (20, 2)" in output + assert "..." in output + + +def test_print_quantity_summarises_higher_dimensional_arrays(capsys): + print_quantity(quantity([[[1, 2], [3, 4]], [[5, 6], [7, 8]]])) + + output = capsys.readouterr().out + assert "3-D array" in output + + +def test_print_quantity_can_omit_the_stats_table(capsys): + print_quantity(quantity([0.0, 0.5, 1.0]), show_stats=False) + + output = capsys.readouterr().out + assert "median" not in output + + +def test_print_quantity_uses_the_label_over_the_name(capsys): + print_quantity(quantity([1, 2, 3], name="grid/rho_tor_norm"), label="field") + + output = capsys.readouterr().out + assert "field" in output + assert "rho_tor_norm" not in output + + +def test_print_quantity_falls_back_to_a_dash_for_missing_units(capsys): + print_quantity(quantity(1.0, units="")) + + assert "[-]" in capsys.readouterr().out + + +def test_stats_are_omitted_for_a_single_value(capsys): + print_quantity(quantity([42.0])) + + output = capsys.readouterr().out + assert "shape (1,)" in output + assert "median" not in output + + +# --------------------------------------------------------------------------- +# show_quantity_textual_plot +# --------------------------------------------------------------------------- + + +def test_plot_is_drawn_for_a_numeric_field(capsys): + show_quantity_textual_plot(quantity([0.0, 1.0, 4.0, 9.0]), label="field") + + output = capsys.readouterr().out + assert "index [-]" in output + assert "shape (4,)" in output + + +def test_plot_uses_a_matching_coordinate_as_the_x_axis(capsys): + show_quantity_textual_plot( + quantity([0.0, 1.0, 4.0, 9.0]), + label="field", + x_quantity=quantity([0, 1, 2, 3], name="profiles_1d/time", units="s"), + ) + + output = capsys.readouterr().out + assert "time [s]" in output + assert "index [-]" not in output + + +def test_plot_ignores_a_coordinate_of_a_different_length(capsys): + show_quantity_textual_plot( + quantity([0.0, 1.0, 4.0]), + x_quantity=quantity([0, 1], name="time", units="s"), + ) + + assert "index [-]" in capsys.readouterr().out + + +def test_non_numeric_data_is_printed_instead_of_plotted(capsys): + show_quantity_textual_plot(quantity(["a", "b"]), label="field") + + output = capsys.readouterr().out + assert "index [-]" not in output + assert "shape (2,)" in output + + +# --------------------------------------------------------------------------- +# print_simulations +# --------------------------------------------------------------------------- + + +def test_print_simulations_reports_an_empty_list(capsys): + print_simulations([]) + + assert "No simulations found" in capsys.readouterr().out + + +def test_print_simulations_prints_a_single_alias_column(capsys): + print_simulations([make_simulation("first"), make_simulation("second")]) + + output = capsys.readouterr().out + assert "alias" in output + assert "first" in output + assert "second" in output + assert "UUID" not in output + assert "status" not in output + + +def test_print_simulations_adds_datetime_and_status_when_verbose(capsys): + print_simulations([make_simulation("first", status="passed")], verbose=True) + + output = capsys.readouterr().out + assert "datetime" in output + assert "status" in output + assert "passed" in output + + +def test_print_simulations_adds_the_uuid_column_on_request(capsys): + print_simulations([make_simulation("first", uuid="abcd1234")], show_uuid=True) + + output = capsys.readouterr().out + assert "UUID" in output + assert "abcd1234" in output + + +def test_print_simulations_adds_a_column_per_metadata_name(capsys): + simulations = [ + make_simulation("first", meta={"pulse": 134173}), + make_simulation("second", meta={}), + ] + + print_simulations(simulations, metadata_names=["pulse"]) + + output = capsys.readouterr().out + assert "pulse" in output + assert "134173" in output + # A simulation without the metadata still gets a row. + assert "second" in output + + +def test_print_simulations_handles_a_missing_alias(capsys): + print_simulations([make_simulation(None, uuid="abcd1234")], show_uuid=True) + + assert "abcd1234" in capsys.readouterr().out + + +def test_print_simulations_hints_at_the_limit_when_a_full_page_is_returned(capsys): + simulations = [make_simulation(f"sim{index}") for index in range(100)] + + print_simulations(simulations) + + assert "first 100 entries shown" in capsys.readouterr().out + + +def test_print_simulations_does_not_hint_below_the_limit(capsys): + print_simulations([make_simulation(f"sim{index}") for index in range(99)]) + + assert "first 100 entries shown" not in capsys.readouterr().out + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ({"min": 1, "max": 5}, "[1, 5]"), + ([1.0, 2.0], "[1.00, 2.00]"), + ([True, False], "[True, False]"), + (["a", "b"], "[a, b]"), + # Long lists are truncated to five entries. + (list(range(10)), "[0.00, 1.00, 2.00, 3.00, 4.00, ...]"), + ("plain", "plain"), + ], +) +def test_metadata_values_are_formatted_for_the_table(capsys, value, expected): + print_simulations( + [make_simulation("sim", meta={"field": value})], metadata_names=["field"] + ) + + assert expected in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# print_trace +# --------------------------------------------------------------------------- + + +def test_print_trace_reports_a_missing_trace(capsys): + print_trace(None) + + assert "No simulations trace found" in capsys.readouterr().out + + +def test_print_trace_prints_the_simulation_and_its_status_date(capsys): + trace = SimulationTraceData( + alias="current", status="passed", passed_on="2024-01-01" + ) + + print_trace(trace) + + output = capsys.readouterr().out + assert "current" in output + assert "passed" in output + assert "Passed on: 2024-01-01" in output + + +def test_print_trace_reports_an_unknown_status(capsys): + print_trace(SimulationTraceData(alias="current")) + + assert "Status: unknown" in capsys.readouterr().out + + +def test_print_trace_indents_the_replaced_simulation(capsys): + trace = SimulationTraceData( + alias="current", + status="passed", + replaces=SimulationTraceData(alias="older", status="deprecated"), + replaces_reason="superseded", + ) + + print_trace(trace) + + output = capsys.readouterr().out + assert "Replaces: (reason: superseded)" in output + assert " Simulation:" in output + assert "older" in output + + +def test_print_trace_handles_a_replacement_without_a_reason(capsys): + trace = SimulationTraceData( + alias="current", replaces=SimulationTraceData(alias="older") + ) + + print_trace(trace) + + output = capsys.readouterr().out + assert "Replaces:" in output + assert "reason" not in output diff --git a/tests/cli/test_cli_manifest_command.py b/tests/cli/test_cli_manifest_command.py index efc96740..6408ab21 100644 --- a/tests/cli/test_cli_manifest_command.py +++ b/tests/cli/test_cli_manifest_command.py @@ -1,54 +1,65 @@ -from unittest import mock +"""Tests for ``simdb manifest``.""" -from click.testing import CliRunner -from utils import config_test_file, create_manifest, get_file_path +import yaml -from simdb.cli.simdb import cli +from simdb.cli.manifest import Manifest -@mock.patch("simdb.cli.commands.manifest.Manifest") -def test_manifest_check_command(manifest): - config_file = config_test_file() - runner = CliRunner() - file_name = create_manifest() - result = runner.invoke( - cli, [f"--config-file={config_file}", "manifest", "check", str(file_name)] - ) - assert result.exception is None - assert "ok" in result.output - assert manifest.load_from_file.called - (args, kwargs) = manifest.load_from_file.call_args - assert str(args[0]) == str(file_name) - assert kwargs == {} - - -def test_manifest_check_command_integration(): - """Integration test that actually runs the manifest check without mocking.""" - config_file = config_test_file() - runner = CliRunner() - file_name = create_manifest() - result = runner.invoke( - cli, [f"--config-file={config_file}", "manifest", "check", str(file_name)] - ) - assert result.exception is None, f"Unexpected exception: {result.exception}" - assert result.exit_code == 0, ( - f"Exit code: {result.exit_code}, Output: {result.output}" - ) +def test_check_accepts_a_valid_manifest(invoke, manifest_file): + result = invoke("manifest", "check", str(manifest_file)) + + assert result.exit_code == 0 assert "ok" in result.output -@mock.patch("simdb.cli.commands.manifest.Manifest") -def test_manifest_create_command(manifest): - config_file = config_test_file() - runner = CliRunner() - file_name = get_file_path("manifest.yaml") - result = runner.invoke( - cli, [f"--config-file={config_file}", "manifest", "create", str(file_name)] - ) - assert result.exception is None - assert str(file_name) in result.output - assert manifest.from_template.called - assert manifest.from_template().save.called - (args, kwargs) = manifest.from_template().save.call_args - assert args[0].name == str(file_name) - assert kwargs == {} +def test_check_rejects_an_invalid_manifest(invoke, tmp_path): + manifest_file = tmp_path / "broken.yaml" + manifest_file.write_text("manifest_version: 2\nalias: 'not a valid alias'\n") + + result = invoke("manifest", "check", str(manifest_file)) + + assert result.exit_code != 0 + assert "illegal characters in alias" in str(result.exception) + + +def test_check_requires_the_file_to_exist(invoke, tmp_path): + result = invoke("manifest", "check", str(tmp_path / "missing.yaml")) + + assert result.exit_code == 2 + assert "does not exist" in result.output + + +def test_create_writes_a_manifest_from_the_template(invoke, tmp_path): + manifest_file = tmp_path / "new-manifest.yaml" + + result = invoke("manifest", "create", str(manifest_file)) + + assert result.exit_code == 0 + assert str(manifest_file) in result.output + assert manifest_file.exists() + assert yaml.safe_load(manifest_file.read_text())["manifest_version"] == 2 + + +def test_a_created_manifest_carries_the_template_placeholders(invoke, tmp_path): + """``create`` writes a skeleton, so ``check`` still has something to report. + + The template points at ``/home/user/path/to/a/file1`` and friends, which the + user is expected to replace; checking it unedited must say so rather than + pass silently. + """ + manifest_file = tmp_path / "new-manifest.yaml" + assert invoke("manifest", "create", str(manifest_file)).exit_code == 0 + + result = invoke("manifest", "check", str(manifest_file)) + + assert result.exit_code != 0 + assert "No files found matching path" in str(result.exception) + + +def test_check_loads_the_manifest_it_was_given(invoke, manifest_file): + """``check`` reports on the requested file, not on a default one.""" + loaded = Manifest.load_from_file(manifest_file) + + assert loaded.alias == "simulation-alias" + assert len(loaded.inputs) == 1 + assert len(loaded.outputs) == 1 diff --git a/tests/cli/test_cli_optional_remote_argument.py b/tests/cli/test_cli_optional_remote_argument.py new file mode 100644 index 00000000..25d9e4a1 --- /dev/null +++ b/tests/cli/test_cli_optional_remote_argument.py @@ -0,0 +1,105 @@ +"""The optional ``REMOTE`` argument of ``simdb simulation`` sub-commands. + +``simulation push``, ``pull``, ``data`` and ``validate`` all take an optional +REMOTE before their required arguments. Click cannot express "optional first +argument", so :class:`OptionalRemoteCommand` fills in an empty REMOTE when the +command line does not provide a value for every argument. + +Counting raw command line entries instead is what made ``push SIM_ID --replaces +=x`` fail with "Missing argument 'SIM_ID'": the option was counted as an +argument, so no empty REMOTE was inserted. +""" + +from unittest import mock + +import pytest +from cli_helpers import make_simulation + +SUBCOMMANDS = [ + ("push", ()), + ("validate", ()), + ("pull", ("directory",)), + ("data", ("core_profiles/time",)), +] +"""Each sub-command and the arguments that follow its SIM_ID.""" + + +@pytest.fixture +def remote_api(): + with mock.patch("simdb.cli.commands.simulation.RemoteAPI") as remote_api_cls: + remote_api_cls.return_value.get_validation_schemas.return_value = [] + yield remote_api_cls + + +@pytest.fixture(autouse=True) +def a_simulation_exists(local_db): + local_db.get_simulation.return_value = make_simulation("sim") + local_db.get_simulation.side_effect = None + return local_db + + +@pytest.mark.parametrize(("subcommand", "trailing"), SUBCOMMANDS) +@pytest.mark.parametrize( + "options", [("--username", "bob"), ()], ids=["username", "no-options"] +) +@pytest.mark.parametrize( + "options_first", [True, False], ids=["options-first", "options-last"] +) +@pytest.mark.parametrize( + "remote", [("test",), ()], ids=["named-remote", "default-remote"] +) +def test_the_remote_may_be_omitted( + invoke, remote_api, remote, options_first, options, subcommand, trailing +): + """REMOTE may be left out, wherever the options appear on the command line.""" + arguments = (*remote, "sim", *trailing) + argv = (*options, *arguments) if options_first else (*arguments, *options) + + result = invoke("simulation", subcommand, *argv) + + assert remote_api.called, result.output + used_remote, used_username = remote_api.call_args.args[:2] + assert used_remote == (remote[0] if remote else "") + assert used_username == ("bob" if options else None) + + +@pytest.mark.parametrize( + ("subcommand", "arguments", "option"), + [ + ("push", ["sim"], "--replaces=older"), + ("push", ["sim"], "--add-watcher"), + ("data", ["sim", "core_profiles/time"], "--dd-version=4.1.1"), + ], +) +def test_a_command_specific_option_does_not_consume_the_remote( + invoke, remote_api, subcommand, arguments, option +): + """Options that are not shared by every sub-command must count the same way.""" + result = invoke("simulation", subcommand, *arguments, option) + + assert "Missing argument" not in result.output + assert remote_api.call_args.args[0] == "" + + +@pytest.mark.parametrize( + ("subcommand", "trailing"), + [(subcommand, trailing) for subcommand, trailing in SUBCOMMANDS if trailing], +) +def test_a_genuinely_missing_argument_is_still_reported( + invoke, remote_api, subcommand, trailing +): + """Filling in the REMOTE must not paper over an argument the user forgot.""" + result = invoke("simulation", subcommand, "sim", *trailing[:-1]) + + assert result.exit_code == 2 + assert "Missing argument" in result.output + assert not remote_api.called + + +@pytest.mark.parametrize("subcommand", [subcommand for subcommand, _ in SUBCOMMANDS]) +def test_a_command_with_no_arguments_at_all_is_reported(invoke, remote_api, subcommand): + result = invoke("simulation", subcommand) + + assert result.exit_code == 2 + assert "Missing argument" in result.output + assert not remote_api.called diff --git a/tests/cli/test_cli_provenance_command.py b/tests/cli/test_cli_provenance_command.py index a68026f7..7d4cfe20 100644 --- a/tests/cli/test_cli_provenance_command.py +++ b/tests/cli/test_cli_provenance_command.py @@ -1,22 +1,48 @@ -from unittest import mock - -from click.testing import CliRunner -from utils import config_test_file, get_file_path - -from simdb.cli.simdb import cli - - -@mock.patch("yaml.dump") -def test_provenance_command(dump): - config_file = config_test_file() - runner = CliRunner() - file_name = get_file_path("provenance.yaml") - result = runner.invoke( - cli, [f"--config-file={config_file}", "provenance", str(file_name)] - ) - assert result.exception is None - assert str(file_name) in result.output - assert dump.called - (args, kwargs) = dump.call_args - assert args[1].name == str(file_name) - assert kwargs == {"default_flow_style": False} +"""Tests for ``simdb provenance``.""" + +import yaml + + +def test_provenance_writes_a_yaml_description_of_the_system(invoke, tmp_path): + provenance_file = tmp_path / "provenance.yaml" + + result = invoke("provenance", str(provenance_file)) + + assert result.exit_code == 0 + assert str(provenance_file) in result.output + + provenance = yaml.safe_load(provenance_file.read_text()) + assert set(provenance) == {"environment", "platform"} + assert provenance["platform"]["system"] + assert provenance["platform"]["python_version"] + + +def test_path_like_environment_variables_are_split_into_lists( + invoke, tmp_path, monkeypatch +): + monkeypatch.setenv("SIMDB_TEST_PATH", "/first:/second") + provenance_file = tmp_path / "provenance.yaml" + + assert invoke("provenance", str(provenance_file)).exit_code == 0 + + environment = yaml.safe_load(provenance_file.read_text())["environment"] + assert environment["SIMDB_TEST_PATH"] == ["/first", "/second"] + + +def test_other_environment_variables_are_kept_as_strings(invoke, tmp_path, monkeypatch): + monkeypatch.setenv("SIMDB_TEST_VALUE", "plain") + provenance_file = tmp_path / "provenance.yaml" + + assert invoke("provenance", str(provenance_file)).exit_code == 0 + + environment = yaml.safe_load(provenance_file.read_text())["environment"] + assert environment["SIMDB_TEST_VALUE"] == "plain" + + +def test_the_file_is_overwritten_on_a_second_run(invoke, tmp_path): + provenance_file = tmp_path / "provenance.yaml" + provenance_file.write_text("stale: true\n") + + assert invoke("provenance", str(provenance_file)).exit_code == 0 + + assert "stale" not in provenance_file.read_text() diff --git a/tests/cli/test_cli_remote_api_client.py b/tests/cli/test_cli_remote_api_client.py new file mode 100644 index 00000000..39116a9c --- /dev/null +++ b/tests/cli/test_cli_remote_api_client.py @@ -0,0 +1,374 @@ +"""Tests for the HTTP layer of :class:`simdb.cli.remote_api.RemoteAPI`. + +The other CLI tests stub ``RemoteAPI`` methods so they can concentrate on the +commands. Here it is the client itself that is under test, so only ``requests`` +is replaced: construction, authentication, URL building, error translation and +response parsing all run for real. +""" + +import json +from unittest import mock + +import pytest +import requests +from pydantic import ValidationError + +from simdb.cli.remote_api import ( + FailedConnection, + RemoteAPI, + RemoteError, + check_return, + try_request, +) +from simdb.config import Config +from simdb.remote.models import TokenResponse + +INDEX = { + "api": "SimDB", + "api_version": "1.3", + "server_version": "0.11", + "endpoints": ["http://remote.test/v1.2", "http://remote.test/v1.3"], + "authentication": "None", +} + + +def response(payload=None, status=200, content=None) -> requests.Response: + """Build a real :class:`requests.Response` around a JSON payload.""" + res = requests.Response() + res.status_code = status + res.url = "http://remote.test/" + res.reason = "OK" if status == 200 else "Error" + if content is None: + content = json.dumps(payload if payload is not None else {}).encode() + res._content = content + return res + + +class FakeHttp: + """Answer ``requests.get``/``post`` from a URL suffix to payload mapping.""" + + def __init__(self, index=None): + self.routes = {"": index if index is not None else dict(INDEX)} + self.calls = [] + + def route(self, suffix, payload=None, status=200, content=None): + self.routes[suffix] = response(payload, status=status, content=content) + + def _respond(self, method, url, **kwargs): + self.calls.append(mock.call(method, url, **kwargs)) + suffix = url.split("/v1.3/", 1)[-1] if "/v1.3/" in url else "" + if url.rstrip("/") in ("http://remote.test", "http://remote.test/"): + suffix = "" + route = self.routes.get(suffix) + if route is None: + return response({"error": f"no route for {url!r}"}, status=404) + return route if isinstance(route, requests.Response) else response(route) + + def request_for(self, suffix): + """The recorded call whose URL ends with the given suffix.""" + for call in self.calls: + if call.args[1].endswith(suffix): + return call + raise AssertionError(f"no request was made to {suffix!r}") + + +@pytest.fixture +def http(monkeypatch): + fake = FakeHttp() + for method in ("get", "post", "put", "delete"): + monkeypatch.setattr( + requests, + method, + lambda url, _method=method, **kwargs: fake._respond(_method, url, **kwargs), + ) + return fake + + +def make_config(**options) -> Config: + config = Config() + config.set_option("remote.test.url", "http://remote.test") + for name, value in options.items(): + config.set_option(f"remote.test.{name}", value) + return config + + +# --------------------------------------------------------------------------- +# construction +# --------------------------------------------------------------------------- + + +def test_the_negotiated_version_is_used_for_requests(http): + api = RemoteAPI("test", None, None, make_config()) + + assert api._api_url == "http://remote.test/v1.3/" + assert api.remote == "test" + assert api.has_url() is True + + +def test_the_default_remote_is_used_when_no_name_is_given(http): + config = make_config() + config.default_remote = "test" + + api = RemoteAPI(None, None, None, config) + + assert api.remote == "test" + + +def test_a_missing_remote_name_without_a_default_is_reported(http): + with pytest.raises(KeyError, match="no default remote"): + RemoteAPI(None, None, None, Config()) + + +def test_an_unknown_remote_is_reported(http): + with pytest.raises(ValueError, match="Remote 'other' not found"): + RemoteAPI("other", None, None, make_config()) + + +def test_a_password_without_a_username_is_rejected(http): + with pytest.raises(ValueError, match="Password given but no username"): + RemoteAPI("test", None, "secret", make_config()) + + +def test_authentication_without_credentials_or_a_token_is_rejected(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + + with pytest.raises(ValueError, match="No username or password given"): + RemoteAPI("test", None, None, make_config()) + + +def test_a_remote_without_a_usable_version_is_reported(http): + http.routes[""] = response({**INDEX, "endpoints": ["http://remote.test/v9"]}) + + with pytest.raises(RemoteError, match="No compatible API version"): + RemoteAPI("test", None, None, make_config()) + + +def test_the_api_version_comes_from_the_negotiated_endpoint(http): + http.routes[""] = response({**INDEX, "api_version": None}) + + api = RemoteAPI("test", None, None, make_config()) + + # The negotiated endpoint decides the version, so a remote that reports no + # api_version of its own is still usable. + assert str(api.version) == "1.3.0" + + +def test_a_remote_that_reports_no_server_version_is_reported(http): + http.routes[""] = response({**INDEX, "server_version": None}) + + with pytest.raises(RemoteError, match="did not report a server version"): + RemoteAPI("test", None, None, make_config()) + + +def test_the_selected_version_is_announced_when_verbose(http, capsys): + config = make_config() + config.verbose = True + + RemoteAPI("test", None, None, config) + + assert "Selected API version v1.3" in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# authentication +# --------------------------------------------------------------------------- + + +def test_no_credentials_are_sent_to_an_unauthenticated_remote(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", []) + + api.get_validation_schemas() + + assert "auth" not in http.request_for("validation_schema").kwargs + + +def test_a_token_is_sent_as_a_jwt_header(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + api = RemoteAPI("test", None, None, make_config(token="123ABC")) + http.route("validation_schema", []) + + api.get_validation_schemas() + + auth = http.request_for("validation_schema").kwargs["auth"] + request = auth(mock.Mock(headers={})) + assert request.headers["Authorization"] == "JWT-Token 123ABC" + + +def test_a_username_and_password_are_sent_as_basic_auth(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + api = RemoteAPI("test", "user", "secret", make_config()) + http.route("validation_schema", []) + + api.get_validation_schemas() + + assert http.request_for("validation_schema").kwargs["auth"] == ("user", "secret") + + +def test_credentials_are_prompted_for_when_the_remote_authenticates(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + + with mock.patch("click.prompt", side_effect=["user", "secret"]) as prompt: + api = RemoteAPI("test", None, None, make_config(), use_token=False) + + assert prompt.call_count == 2 + assert api._username == "user" + assert api._password == "secret" + + +# --------------------------------------------------------------------------- +# requests and responses +# --------------------------------------------------------------------------- + + +def test_requests_are_sent_to_the_versioned_url(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("token", {"token": "NEWTOKEN"}) + + assert api.get_token() == "NEWTOKEN" + assert http.request_for("token").args[1] == "http://remote.test/v1.3/token" + + +def test_the_api_version_context_redirects_requests(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", []) + + with api.api_version("v1.2"): + assert api._api_url == "http://remote.test/v1.2/" + + assert api._api_url == "http://remote.test/v1.3/" + + +def test_list_simulations_sends_the_pagination_headers(http): + api = RemoteAPI("test", None, None, make_config()) + http.route( + "simulations", + { + "count": 1, + "page": 1, + "limit": 10, + "results": [ + { + "uuid": {"_type": "uuid.UUID", "hex": "0" * 32}, + "alias": "sim", + "datetime": "2000-01-01", + } + ], + }, + ) + + simulations = api.list_simulations(limit=10) + + assert [simulation.alias for simulation in simulations] == ["sim"] + headers = http.request_for("simulations").kwargs["headers"] + assert headers["simdb-result-limit"] == "10" + + +def test_list_simulations_appends_the_requested_metadata(http): + api = RemoteAPI("test", None, None, make_config()) + http.route( + "simulations?pulse&run", {"count": 0, "page": 1, "limit": 0, "results": []} + ) + + api.list_simulations(meta=["pulse", "run"]) + + assert http.request_for("simulations?pulse&run") + + +def test_an_error_payload_becomes_a_remote_error(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", {"error": "you shall not pass"}, status=403) + + with pytest.raises(RemoteError, match="you shall not pass"): + api.get_validation_schemas() + + +def test_an_error_without_a_payload_becomes_a_failed_connection(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", status=500, content=b"oops") + + with pytest.raises(FailedConnection, match="HTTP error 500"): + api.get_validation_schemas() + + +def test_a_non_json_response_becomes_a_failed_connection(http): + """A firewall login page is HTML where the API promised JSON.""" + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", content=b"login") + + with pytest.raises(FailedConnection, match="Invalid JSON"): + api.get_validation_schemas() + + +def test_unexpected_json_becomes_a_remote_error(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("token", {"not_a_token": True}) + + with pytest.raises(RemoteError, match="Unexpected data exchanged"): + api.get_token() + + +# --------------------------------------------------------------------------- +# check_return and try_request, tested on their own +# --------------------------------------------------------------------------- + + +def test_check_return_accepts_a_successful_response(): + assert check_return(response({"ok": True})) is None + + +def test_check_return_raises_for_status_without_an_error_field(): + with pytest.raises(requests.HTTPError): + check_return(response({"detail": "nope"}, status=404)) + + +def test_try_request_translates_a_connection_error(): + request = mock.Mock(url="http://remote.test/v1.3/simulations") + + @try_request + def failing(): + raise requests.ConnectionError(request=request) + + with pytest.raises(FailedConnection, match="Connection failed to"): + failing() + + +def test_try_request_reports_an_unknown_url_for_a_request_less_error(): + @try_request + def failing(): + raise requests.ConnectionError(request=None) + + with pytest.raises(FailedConnection, match="undefined"): + failing() + + +def test_try_request_translates_a_json_decode_error(): + @try_request + def failing(): + raise requests.JSONDecodeError("bad", "doc", 0) + + with pytest.raises(FailedConnection, match="Invalid JSON"): + failing() + + +def test_try_request_translates_a_validation_error(): + @try_request + def failing(): + TokenResponse.model_validate({"wrong": "shape"}) + + with pytest.raises(RemoteError, match="Unexpected data exchanged"): + failing() + + +def test_try_request_passes_a_successful_call_through(): + @try_request + def succeeding(value): + return value + + assert succeeding(42) == 42 + + +def test_a_validation_error_is_not_swallowed_as_something_else(): + """Only ``json_invalid`` errors mean the response was not JSON at all.""" + with pytest.raises(ValidationError): + TokenResponse.model_validate_json(b"{}") diff --git a/tests/cli/test_cli_remote_command.py b/tests/cli/test_cli_remote_command.py index 00055fad..b03d7941 100644 --- a/tests/cli/test_cli_remote_command.py +++ b/tests/cli/test_cli_remote_command.py @@ -1,311 +1,469 @@ +"""Tests for ``simdb remote``. + +The remote handshake is stubbed by the ``remote_handshake`` fixture; each test +stubs only the one API call its command makes. +""" + +import uuid from unittest import mock -from click.testing import CliRunner -from utils import config_test_file +import pytest +from cli_helpers import make_simulation -from simdb.cli.simdb import cli +from simdb.cli.remote_api import RemoteAPI +from simdb.database.models.simulation import Simulation from simdb.notifications import Notification -from simdb.remote.models import WatcherData - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_watchers") -def test_remote_watchers_list_command( - list_watchers, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - sim_id = "acbd1234" +from simdb.remote.models import SimulationTraceData, WatcherData + +pytestmark = pytest.mark.usefixtures("remote_handshake") + + +@pytest.fixture +def api_call(): + """Stub a single :class:`RemoteAPI` method by name. + + ``api_call("list_simulations", return_value=[...])`` patches the method for + the duration of the test and hands back the mock to assert on. + """ + patches = [] + + def _api_call(name, **kwargs): + patcher = mock.patch.object(RemoteAPI, name, **kwargs) + patches.append(patcher) + return patcher.start() + + yield _api_call + + for patcher in reversed(patches): + patcher.stop() + + +# --------------------------------------------------------------------------- +# remote test / directory +# --------------------------------------------------------------------------- + + +def test_test_command_reports_the_remote_api_version(invoke, remote_handshake): + result = invoke("remote", "test") + + assert result.exit_code == 0 + assert "Remote is valid" in result.output + assert "1.3" in result.output + + +def test_directory_prints_the_remote_storage_directory(invoke, api_call): + api_call("get_directory", return_value="/srv/simdb/data") + + result = invoke("remote", "directory") + + assert result.exit_code == 0 + assert "/srv/simdb/data" in result.output + + +# --------------------------------------------------------------------------- +# remote watcher +# --------------------------------------------------------------------------- + + +def test_watcher_list_prints_every_watcher(invoke, api_call, remote_handshake): watchers = [ WatcherData(username="a", email="a@simdb.test", notification="A"), WatcherData(username="b", email="b@simdb.test", notification="V"), WatcherData(username="c", email="c@simdb.test", notification="R"), ] - list_watchers.return_value = watchers - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "watcher", "list", sim_id] - ) - assert result.exception is None - assert sim_id in result.output + list_watchers = api_call("list_watchers", return_value=watchers) + + result = invoke("remote", "watcher", "list", "acbd1234") + + assert result.exit_code == 0 + assert "acbd1234" in result.output for watcher in watchers: assert watcher.username in result.output assert watcher.email in result.output assert list_watchers.called + assert remote_handshake.get_endpoints.called -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.remove_watcher") -def test_remote_watcher_remove_command( - remove_watcher, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - user = "test" - sim_id = "acbd1234" - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, - [ - f"--config-file={config_file}", - "remote", - "watcher", - "remove", - sim_id, - f"--user={user}", - ], +def test_watcher_list_reports_no_watchers(invoke, api_call): + api_call("list_watchers", return_value=[]) + + result = invoke("remote", "watcher", "list", "acbd1234") + + assert result.exit_code == 0 + assert "no watchers found for simulation acbd1234" in result.output + + +def test_watcher_remove_passes_the_user_on(invoke, api_call): + remove_watcher = api_call("remove_watcher") + + result = invoke("remote", "watcher", "remove", "acbd1234", "--user=test") + + assert result.exit_code == 0 + assert "acbd1234" in result.output + assert remove_watcher.call_args.args == ("acbd1234", "test") + + +def test_watcher_remove_fails_without_a_user(invoke, api_call): + remove_watcher = api_call("remove_watcher") + + result = invoke("remote", "watcher", "remove", "acbd1234") + + assert result.exit_code != 0 + assert not remove_watcher.called + + +@pytest.mark.xfail( + strict=True, + reason=( + "remove_watcher calls get_string_option('user.name') without default=None, " + "so the config lookup raises KeyError before its own error message is built" + ), +) +def test_watcher_remove_explains_a_missing_user(invoke, api_call): + """``watcher add`` reports this cleanly; ``watcher remove`` should match it.""" + api_call("remove_watcher") + + result = invoke("remote", "watcher", "remove", "acbd1234") + + assert "User not provided and user.name not found in config" in result.output + + +def test_watcher_add_passes_user_email_and_notification(invoke, api_call): + add_watcher = api_call("add_watcher") + + result = invoke( + "remote", + "watcher", + "add", + "acbd1234", + "--user=test", + "--email=test@iter.org", + "--notification=all", ) - assert result.exception is None - assert sim_id in result.output - assert remove_watcher.called - (args, kwargs) = remove_watcher.call_args - assert args == (sim_id, user) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.add_watcher") -def test_remote_watcher_add_command( - add_watcher, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - user = "test" - email = "test@iter.org" - sim_id = "acbd1234" - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, - [ - f"--config-file={config_file}", - "remote", - "watcher", - "add", - sim_id, - f"--user={user}", - f"--email={email}", - "--notification=all", - ], + + assert result.exit_code == 0 + assert "acbd1234" in result.output + assert add_watcher.call_args.args == ( + "acbd1234", + "test", + "test@iter.org", + Notification.ALL, ) - assert result.exception is None - assert sim_id in result.output - assert add_watcher.called - (args, kwargs) = add_watcher.call_args - assert args == (sim_id, user, email, Notification.ALL) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -def test_remote_list_command( - list_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "test"), - ("abcd5678", "test"), - ("abcd4321", "test"), - ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sims.append(sim) - list_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "list", "--uuid"] + + +def test_watcher_add_needs_an_email(invoke, api_call): + add_watcher = api_call("add_watcher") + + result = invoke("remote", "watcher", "add", "acbd1234", "--user=test") + + assert result.exit_code != 0 + assert "Email not provided and user.email not found in config" in result.output + assert not add_watcher.called + + +def test_watcher_add_rejects_an_unknown_notification(invoke, api_call): + add_watcher = api_call("add_watcher") + + result = invoke( + "remote", "watcher", "add", "acbd1234", "--user=t", "-e=t@x", "-n=sometimes" ) - assert result.exception is None - assert list_simulations.called - for el in data: - for i in el: - assert i in result.output - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -def test_remote_list_command_with_verbose( - list_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "test", "2000-01-01-01", "Validated"), - ("abcd5678", "test", "2000-02-02-02", "Validated"), - ("abcd4321", "test", "2000-03-03-03", "Validated"), + + assert result.exit_code == 2 + assert not add_watcher.called + + +# --------------------------------------------------------------------------- +# remote list / info / query +# --------------------------------------------------------------------------- + + +def test_list_prints_the_returned_simulations(invoke, api_call, remote_handshake): + simulations = [ + make_simulation("test", uuid="abcd1234"), + make_simulation("test", uuid="abcd5678"), + make_simulation("test", uuid="abcd4321"), ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sim.datetime = el[2] - sim.status = el[3] - sims.append(sim) - list_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "--verbose", "remote", "list", "--uuid"] - ) - assert result.exception is None + list_simulations = api_call("list_simulations", return_value=simulations) + + result = invoke("remote", "list", "--uuid") + + assert result.exit_code == 0 + for simulation in simulations: + assert simulation.uuid in result.output assert list_simulations.called - for el in data: - for i in el: - assert i in result.output - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_simulation") -def test_remote_info_command( - get_simulation, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - sim_id = "abcd1234" - sim = ("abcd1234", "test", "2000-01-01-01", "Validated") - get_simulation.return_value = sim - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "info", sim_id] + assert remote_handshake.get_endpoints.called + + +def test_list_shows_the_extra_columns_when_verbose(invoke, api_call): + api_call( + "list_simulations", + return_value=[ + make_simulation( + "test", uuid="abcd1234", datetime="2000-01-01-01", status="passed" + ) + ], ) - assert result.exception is None - assert str(sim) in result.output - assert get_simulation.called - (args, kwargs) = get_simulation.call_args - assert args == (sim_id,) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.query_simulations") -def test_remote_query_command( - query_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "123"), - ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sim.find_meta.return_value = [] - sims.append(sim) + + result = invoke("--verbose", "remote", "list", "--uuid") + + assert result.exit_code == 0 + assert "2000-01-01-01" in result.output + assert "passed" in result.output + + +def test_list_rejects_a_negative_limit(invoke, api_call): + list_simulations = api_call("list_simulations") + + result = invoke("remote", "list", "--limit=-1") + + assert result.exit_code == 2 + assert not list_simulations.called + + +def test_info_prints_the_simulation(invoke, api_call): + get_simulation = api_call("get_simulation", return_value="simulation description") + + result = invoke("remote", "info", "abcd1234") + + assert result.exit_code == 0 + assert "simulation description" in result.output + assert get_simulation.call_args.args == ("abcd1234",) + + +def test_query_forwards_the_constraints(invoke, api_call): constraints = ("alias=123", "description=in:test") - query_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "query", "--uuid", *constraints] + query_simulations = api_call( + "query_simulations", return_value=[make_simulation("123", uuid="abcd1234")] ) - assert result.exception is None - for el in data: - for i in el: - assert i in result.output - assert query_simulations.called - (args, kwargs) = query_simulations.call_args - assert args == (constraints, (), 100) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.query_simulations") -def test_remote_query_command_with_verbose( - query_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "123", "2000-01-01-01", "Validated"), - ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sim.datetime = el[2] - sim.status = el[3] - sim.find_meta.return_value = [] - sims.append(sim) - constraints = ("alias=123", "description=in:test") - query_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, - [ - f"--config-file={config_file}", - "--verbose", - "remote", - "query", - "--uuid", - *constraints, + + result = invoke("remote", "query", "--uuid", *constraints) + + assert result.exit_code == 0 + assert "abcd1234" in result.output + assert query_simulations.call_args.args == (constraints, (), 100) + + +def test_query_shows_the_extra_columns_when_verbose(invoke, api_call): + api_call( + "query_simulations", + return_value=[ + make_simulation( + "123", uuid="abcd1234", datetime="2000-01-01-01", status="passed" + ) ], ) - assert result.exception is None - for el in data: - for i in el: - assert i in result.output - assert query_simulations.called - (args, kwargs) = query_simulations.call_args - assert args == (constraints, (), 100) - assert kwargs == {} + + result = invoke("--verbose", "remote", "query", "--uuid", "alias=123") + + assert result.exit_code == 0 + assert "2000-01-01-01" in result.output + assert "passed" in result.output + + +# --------------------------------------------------------------------------- +# remote version / trace +# --------------------------------------------------------------------------- + + +def test_version_prints_the_remote_simdb_version(invoke): + result = invoke("remote", "version") + + assert result.exit_code == 0 + assert "Remote 'test' SimDB version: 0.11" in result.output + + +def test_trace_prints_the_provenance_chain(invoke, api_call): + trace_simulation = api_call( + "trace_simulation", + return_value=SimulationTraceData( + alias="current", + status="passed", + replaces=SimulationTraceData(alias="older", status="deprecated"), + ), + ) + + result = invoke("remote", "trace", "abcd1234") + + assert result.exit_code == 0 + assert "current" in result.output + assert "older" in result.output + assert trace_simulation.call_args.args == ("abcd1234",) + + +def test_trace_reports_a_missing_trace(invoke, api_call): + api_call("trace_simulation", return_value=None) + + result = invoke("remote", "trace", "abcd1234") + + assert result.exit_code == 0 + assert "No simulations trace found" in result.output + + +# --------------------------------------------------------------------------- +# remote schema +# --------------------------------------------------------------------------- + + +def test_schema_prints_the_validation_schemas(invoke, api_call): + api_call("get_validation_schemas", return_value=[{"alias": {"required": True}}]) + + result = invoke("remote", "schema") + + assert result.exit_code == 0 + assert "alias" in result.output + + +def test_schema_rejects_a_non_positive_depth(invoke, api_call): + get_validation_schemas = api_call("get_validation_schemas", return_value=[]) + + result = invoke("remote", "schema", "--depth=0") + + assert result.exit_code == 2 + assert "must be greater than zero" in result.output + assert not get_validation_schemas.called + + +# --------------------------------------------------------------------------- +# remote token +# --------------------------------------------------------------------------- + + +def test_token_new_stores_the_token_in_the_configuration(invoke, api_call, config_file): + api_call("get_token", return_value="NEWTOKEN") + + result = invoke("remote", "token", "new") + + assert result.exit_code == 0 + assert "Token added for remote test." in result.output + assert "NEWTOKEN" in config_file.read_text() + + +def test_token_delete_removes_the_token(invoke, config_file): + result = invoke("remote", "token", "delete") + + assert result.exit_code == 0 + assert "Token for remote test deleted." in result.output + assert "123ABC" not in config_file.read_text() + + +def test_token_delete_can_be_repeated(invoke): + assert invoke("remote", "token", "delete").exit_code == 0 + + result = invoke("remote", "token", "delete") + + assert result.exit_code == 0 + + +@pytest.mark.xfail( + strict=True, + reason=( + "Config.delete_option only raises KeyError for a missing section; " + "configparser.remove_option returns False for a missing option instead " + "of raising, so the deletion is reported as successful" + ), +) +def test_token_delete_says_when_there_was_no_token(invoke): + assert invoke("remote", "token", "delete").exit_code == 0 + + result = invoke("remote", "token", "delete") + + assert "No token for remote test found." in result.output + + +# --------------------------------------------------------------------------- +# remote admin +# --------------------------------------------------------------------------- + + +def test_admin_set_meta_reports_an_update(invoke, api_call): + set_metadata = api_call("set_metadata", return_value="old") + + result = invoke("remote", "admin", "set-meta", "abcd1234", "pulse", "134173") + + assert result.exit_code == 0 + assert "old -> 134173" in result.output + assert set_metadata.call_args.args == ("abcd1234", "pulse", "134173") + + +def test_admin_set_meta_reports_a_new_value(invoke, api_call): + api_call("set_metadata", return_value=None) + + result = invoke("remote", "admin", "set-meta", "abcd1234", "pulse", "134173") + + assert result.exit_code == 0 + assert "Added pulse for simulation abcd1234 with value '134173'" in result.output + + +@pytest.mark.parametrize( + ("type_name", "value", "expected"), + [ + ("int", "42", 42), + ("float", "1.5", 1.5), + ("string", "42", "42"), + ], +) +def test_admin_set_meta_converts_the_value( + invoke, api_call, type_name, value, expected +): + set_metadata = api_call("set_metadata", return_value=None) + + result = invoke( + "remote", "admin", "set-meta", "abcd1234", "key", value, f"--type={type_name}" + ) + + assert result.exit_code == 0 + assert set_metadata.call_args.args[2] == expected + + +def test_admin_set_meta_converts_a_uuid(invoke, api_call): + set_metadata = api_call("set_metadata", return_value=None) + value = "12345678123456781234567812345678" + + result = invoke( + "remote", "admin", "set-meta", "abcd1234", "key", value, "--type=UUID" + ) + + assert result.exit_code == 0 + assert set_metadata.call_args.args[2] == uuid.UUID(value) + + +def test_admin_set_status_updates_the_status(invoke, api_call): + update_simulation = api_call("update_simulation", return_value="not validated") + + result = invoke("remote", "admin", "set-status", "abcd1234", "PASSED") + + assert result.exit_code == 0 + assert "not validated -> PASSED" in result.output + assert update_simulation.call_args.args == ( + "abcd1234", + Simulation.Status.PASSED, + ) + + +def test_admin_set_status_rejects_an_unknown_status(invoke, api_call): + update_simulation = api_call("update_simulation") + + result = invoke("remote", "admin", "set-status", "abcd1234", "excellent") + + assert result.exit_code == 2 + assert not update_simulation.called + + +def test_admin_del_meta_removes_the_key(invoke, api_call): + delete_metadata = api_call("delete_metadata") + + result = invoke("remote", "admin", "del-meta", "abcd1234", "pulse") + + assert result.exit_code == 0 + assert "Deleted pulse for simulation abcd1234" in result.output + assert delete_metadata.call_args.args == ("abcd1234", "pulse") + + +def test_admin_delete_removes_the_simulation(invoke, api_call): + delete_simulation = api_call("delete_simulation") + + result = invoke("remote", "admin", "delete", "abcd1234") + + assert result.exit_code == 0 + assert "Deleted simulation abcd1234" in result.output + assert delete_simulation.call_args.args == ("abcd1234",) diff --git a/tests/cli/test_cli_remote_config_command.py b/tests/cli/test_cli_remote_config_command.py new file mode 100644 index 00000000..02751570 --- /dev/null +++ b/tests/cli/test_cli_remote_config_command.py @@ -0,0 +1,152 @@ +"""Tests for ``simdb remote config``. + +These commands only touch the configuration file, so no remote is involved. The +file they write back to is the throw-away one the ``config_file`` fixture +creates inside ``tmp_path``. +""" + +import configparser + +import pytest + + +@pytest.fixture +def saved_config(config_file): + """Read back what ``Config.save`` wrote. + + ``Config.load`` treats a ``--config-file`` as the user configuration, so + that is the file ``save`` writes back to. + """ + + def _saved_config() -> configparser.ConfigParser: + parser = configparser.ConfigParser() + parser.read(config_file) + return parser + + return _saved_config + + +def test_list_shows_the_configured_remotes(invoke): + result = invoke("remote", "config", "list") + + assert result.exit_code == 0 + assert "test: http://0.0.0.0:5000/ (default)" in result.output + + +def test_list_shows_the_username_and_firewall_of_a_remote(invoke): + assert ( + invoke("remote", "config", "set-option", "test", "username", "me").exit_code + == 0 + ) + assert ( + invoke("remote", "config", "set-option", "test", "firewall", "F5").exit_code + == 0 + ) + + result = invoke("remote", "config", "list") + + assert result.exit_code == 0 + assert "firewall: F5" in result.output + assert "username: me" in result.output + + +def test_default_prints_the_default_remote(invoke): + result = invoke("remote", "config", "default") + + assert result.exit_code == 0 + assert result.output.strip() == "test" + + +def test_get_default_prints_the_default_remote(invoke): + result = invoke("remote", "config", "get-default") + + assert result.exit_code == 0 + assert result.output.strip() == "test" + + +def test_new_adds_a_remote(invoke, saved_config): + result = invoke("remote", "config", "new", "other", "http://other.test") + + assert result.exit_code == 0 + assert saved_config()['remote "other"']["url"] == "http://other.test" + + +def test_new_can_record_a_username_and_firewall(invoke, saved_config): + result = invoke( + "remote", + "config", + "new", + "other", + "http://other.test", + "--username=me", + "--firewall=F5", + ) + + assert result.exit_code == 0 + section = saved_config()['remote "other"'] + assert section["username"] == "me" + assert section["firewall"] == "F5" + + +def test_new_rejects_an_unknown_firewall(invoke): + result = invoke( + "remote", "config", "new", "other", "http://other.test", "--firewall=nope" + ) + + assert result.exit_code == 2 + + +def test_new_can_make_the_remote_the_default(invoke, saved_config): + result = invoke( + "remote", "config", "new", "other", "http://other.test", "--default" + ) + + assert result.exit_code == 0 + config = saved_config() + assert config.getboolean('remote "other"', "default") is True + assert config.getboolean('remote "test"', "default") is False + + +def test_set_default_switches_the_default_remote(invoke, saved_config): + assert ( + invoke("remote", "config", "new", "other", "http://other.test").exit_code == 0 + ) + + result = invoke("remote", "config", "set-default", "other") + + assert result.exit_code == 0 + assert saved_config().getboolean('remote "other"', "default") is True + + +def test_delete_removes_a_remote(invoke, saved_config): + assert ( + invoke("remote", "config", "new", "other", "http://other.test").exit_code == 0 + ) + + result = invoke("remote", "config", "delete", "other") + + assert result.exit_code == 0 + assert 'remote "other"' not in saved_config().sections() + + +def test_set_option_stores_an_arbitrary_option(invoke, saved_config): + result = invoke("remote", "config", "set-option", "test", "token", "NEWTOKEN") + + assert result.exit_code == 0 + assert saved_config()['remote "test"']["token"] == "NEWTOKEN" + + +@pytest.mark.parametrize( + ("subcommand", "arguments"), + [ + ("new", ["only-a-name"]), + ("delete", []), + ("set-default", []), + ("set-option", ["test", "token"]), + ], +) +def test_missing_arguments_are_reported(invoke, subcommand, arguments): + result = invoke("remote", "config", subcommand, *arguments) + + assert result.exit_code == 2 + assert "Missing argument" in result.output diff --git a/tests/cli/test_cli_root.py b/tests/cli/test_cli_root.py new file mode 100644 index 00000000..99000ef5 --- /dev/null +++ b/tests/cli/test_cli_root.py @@ -0,0 +1,133 @@ +"""Tests for the top level ``simdb`` group in :mod:`simdb.cli.simdb`.""" + +from unittest import mock + +import pytest +from cli_helpers import make_simulation + +from simdb import __version__ +from simdb.cli import simdb as simdb_cli +from simdb.cli.simdb import cli + + +def test_version_option_reports_the_package_version(runner): + result = runner.invoke(cli, ["--version"]) + + assert result.exit_code == 0 + assert __version__ in result.output + + +def test_help_lists_every_command(runner): + result = runner.invoke(cli, ["--help"]) + + assert result.exit_code == 0 + commands = ("alias", "config", "manifest", "provenance", "remote", "simulation") + for command in commands: + assert command in result.output + + +def test_commands_are_listed_in_alphabetical_order(): + listed = cli.list_commands(None) + + assert listed == sorted(listed) + assert "sim" in listed + + +def test_sim_is_an_alias_for_simulation(runner): + result = runner.invoke(cli, ["--help"]) + + assert "Alias for simulation." in result.output + assert cli.get_command(None, "sim") is not None + + +def test_the_alias_reaches_the_same_command(invoke, local_db): + local_db.list_simulations.return_value = [] + + assert invoke("sim", "list").exit_code == 0 + assert local_db.list_simulations.called + + +def test_unknown_commands_are_rejected(runner): + result = runner.invoke(cli, ["nonsense"]) + + assert result.exit_code == 2 + + +def test_hidden_dump_help_prints_help_for_every_subcommand(runner): + result = runner.invoke(cli, ["dump-help"]) + + assert result.exit_code == 0 + # Both a group and one of its sub-commands are documented. + assert "Manage ingested simulations." in result.output + assert "List ingested simulations." in result.output + assert "Query/update application configuration." in result.output + + +def test_dump_help_is_hidden_from_the_command_list(runner): + assert "dump-help" not in runner.invoke(cli, ["--help"]).output + + +def test_config_file_option_is_loaded(invoke): + """The remote declared in the config file is visible to the commands.""" + result = invoke("remote", "config", "list") + + assert result.exit_code == 0 + assert "test: http://0.0.0.0:5000/" in result.output + assert "(default)" in result.output + + +def test_a_missing_config_file_is_reported(runner, tmp_path): + result = runner.invoke( + cli, [f"--config-file={tmp_path / 'nope.cfg'}", "config", "path"] + ) + + assert result.exit_code == 2 + assert "No such file or directory" in result.output + + +def test_verbose_flag_reaches_the_commands(invoke, local_db): + """``--verbose`` is what turns on the extra columns of ``simulation list``.""" + local_db.list_simulations.return_value = [make_simulation("sim")] + + assert "status" not in invoke("simulation", "list").output + assert "status" in invoke("--verbose", "simulation", "list").output + + +class TestMain: + """``main`` is the console-script entry point and the CLI's last error handler.""" + + def test_successful_runs_do_not_raise(self): + with mock.patch.object(simdb_cli, "cli") as command: + simdb_cli.main() + + assert command.called + + def test_errors_are_reported_and_exit_non_zero(self, capsys): + with mock.patch.object( + simdb_cli, "cli", side_effect=RuntimeError("boom") + ), pytest.raises(SystemExit) as exit_info: + simdb_cli.main() + + assert exit_info.value.code == 1 + assert "Error: boom" in capsys.readouterr().err + + def test_debug_mode_re_raises_for_a_traceback(self, monkeypatch): + monkeypatch.setattr(simdb_cli, "g_debug", True) + + with mock.patch.object( + simdb_cli, "cli", side_effect=RuntimeError("boom") + ), pytest.raises(RuntimeError, match="boom"): + simdb_cli.main() + + def test_the_debug_flag_switches_debug_mode_on(self, invoke): + """``-d`` is what makes :func:`main` re-raise instead of printing. + + ``--help`` is eager and exits before the group callback runs, so the + flag only takes effect when a command is actually invoked. + """ + invoke("-d", "config", "path") + assert simdb_cli.g_debug is True + + # Leave the module global as the other tests expect to find it. + invoke("config", "path") + assert simdb_cli.g_debug is False diff --git a/tests/cli/test_cli_simulation_command.py b/tests/cli/test_cli_simulation_command.py index 0120fc02..7541cca1 100644 --- a/tests/cli/test_cli_simulation_command.py +++ b/tests/cli/test_cli_simulation_command.py @@ -1,87 +1,496 @@ +"""Tests for ``simdb simulation``. + +The local database and the remote are stubbed; what is exercised here is the +command layer itself: argument parsing, the branches each command takes, the +calls it makes, and what it reports back to the user. +""" + from unittest import mock -from click.testing import CliRunner -from utils import config_test_file - -from simdb.cli.simdb import cli - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_alias_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_delete_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_info_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_list_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_modify_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_new_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_push_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_query_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_validate_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None +import pytest +from cli_helpers import make_simulation + +from simdb.cli.remote_api import RemoteError +from simdb.database import DatabaseError +from simdb.remote.models import ImasDataResponse, QuantityData +from simdb.validation import ValidationError + + +@pytest.fixture +def remote_api(): + """Replace the ``RemoteAPI`` the simulation commands construct.""" + with mock.patch("simdb.cli.commands.simulation.RemoteAPI") as remote_api_cls: + yield remote_api_cls.return_value + + +# --------------------------------------------------------------------------- +# simulation list +# --------------------------------------------------------------------------- + + +def test_list_prints_every_alias(invoke, local_db): + local_db.list_simulations.return_value = [ + make_simulation("first"), + make_simulation("second"), + ] + + result = invoke("simulation", "list") + + assert result.exit_code == 0 + assert "first" in result.output + assert "second" in result.output + assert local_db.list_simulations.call_args.kwargs == { + "meta_keys": (), + "limit": 100, + } + + +def test_list_reports_an_empty_database(invoke, local_db): + local_db.list_simulations.return_value = [] + + result = invoke("simulation", "list") + + assert result.exit_code == 0 + assert "No simulations found" in result.output + + +def test_list_shows_uuid_and_metadata_columns_on_request(invoke, local_db): + local_db.list_simulations.return_value = [ + make_simulation("first", uuid="abcd1234", meta={"pulse": 134173}) + ] + + result = invoke("simulation", "list", "--uuid", "--meta-data=pulse") + + assert result.exit_code == 0 + assert "abcd1234" in result.output + assert "134173" in result.output + assert local_db.list_simulations.call_args.kwargs["meta_keys"] == ("pulse",) + + +def test_list_passes_the_requested_limit(invoke, local_db): + local_db.list_simulations.return_value = [] + + assert invoke("simulation", "list", "--limit=5").exit_code == 0 + + assert local_db.list_simulations.call_args.kwargs["limit"] == 5 + + +def test_list_rejects_a_negative_limit(invoke, local_db): + result = invoke("simulation", "list", "--limit=-1") + + assert result.exit_code == 2 + assert "must be non-negative" in result.output + assert not local_db.list_simulations.called + + +# --------------------------------------------------------------------------- +# simulation modify +# --------------------------------------------------------------------------- + + +def test_modify_sets_a_new_alias(invoke, local_db): + simulation = make_simulation("old") + local_db.get_simulation.return_value = simulation + + result = invoke("simulation", "modify", "old", "--alias=new") + + assert result.exit_code == 0 + assert "alias updated" in result.output + assert simulation.alias == "new" + assert local_db.session.commit.called + + +def test_modify_sets_metadata(invoke, local_db): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + + result = invoke("simulation", "modify", "sim", "--set-meta=pulse=134173") + + assert result.exit_code == 0 + assert "metadata updated" in result.output + simulation.set_meta.assert_called_once_with("pulse", "134173") + assert local_db.session.commit.called + + +def test_modify_rejects_metadata_without_a_value(invoke, local_db): + result = invoke("simulation", "modify", "sim", "--set-meta=pulse") + + assert result.exit_code == 2 + assert "must be of form NAME=VALUE" in result.output + assert not local_db.session.commit.called + + +def test_modify_deletes_metadata(invoke, local_db): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + + result = invoke("simulation", "modify", "sim", "--del-meta=pulse") + + assert result.exit_code == 0 + assert "metadata deleted" in result.output + simulation.remove_meta.assert_called_once_with("pulse") + + +def test_modify_without_options_changes_nothing(invoke, local_db): + result = invoke("simulation", "modify", "sim") + + assert result.exit_code == 0 + assert "nothing to do" in result.output + assert not local_db.get_simulation.called + + +# --------------------------------------------------------------------------- +# simulation delete +# --------------------------------------------------------------------------- + + +def test_delete_removes_a_single_simulation(invoke, local_db): + simulation = make_simulation("sim") + simulation.uuid = mock.Mock(hex="abcd1234") + local_db.delete_simulation.return_value = simulation + + result = invoke("simulation", "delete", "sim") + + assert result.exit_code == 0 + assert "abcd1234 deleted" in result.output + local_db.delete_simulation.assert_called_once_with("sim") + + +def test_delete_requires_a_simulation_or_all(invoke, local_db): + result = invoke("simulation", "delete") + + assert result.exit_code != 0 + assert "Either SIM_ID or --all must be provided" in result.output + + +def test_delete_all_removes_the_database_file_once_confirmed( + invoke, local_db, tmp_path +): + database_file = tmp_path / "sim.db" + database_file.write_text("not really a database") + + with mock.patch( + "simdb.cli.commands.simulation.Confirm.ask", return_value=True + ) as ask: + result = invoke("simulation", "delete", "--all") + + assert result.exit_code == 0 + assert "Local database reset." in result.output + assert ask.called + assert not database_file.exists() + + +def test_delete_all_keeps_the_database_when_not_confirmed(invoke, local_db, tmp_path): + database_file = tmp_path / "sim.db" + database_file.write_text("not really a database") + + with mock.patch("simdb.cli.commands.simulation.Confirm.ask", return_value=False): + result = invoke("simulation", "delete", "--all") + + # Declining the reset falls through to the single-simulation path, which has + # no SIM_ID to work with. + assert result.exit_code != 0 + assert "Either SIM_ID or --all must be provided" in result.output + assert database_file.exists() + + +# --------------------------------------------------------------------------- +# simulation info +# --------------------------------------------------------------------------- + + +def test_info_prints_the_simulation(invoke, local_db): + local_db.get_simulation.return_value = "simulation description" + + result = invoke("simulation", "info", "sim") + + assert result.exit_code == 0 + assert "simulation description" in result.output + local_db.get_simulation.assert_called_once_with("sim") + + +def test_info_fails_when_the_simulation_is_unknown(invoke, local_db): + local_db.get_simulation.return_value = None + + result = invoke("simulation", "info", "sim") + + assert result.exit_code != 0 + assert isinstance(result.exception, KeyError) + + +# --------------------------------------------------------------------------- +# simulation ingest +# --------------------------------------------------------------------------- + + +def test_ingest_stores_the_manifest_and_reports_the_alias( + invoke, local_db, manifest_file +): + result = invoke("simulation", "ingest", str(manifest_file)) + + assert result.exit_code == 0 + assert "ALIAS: simulation-alias" in result.output + assert local_db.insert_simulation.called + (simulation,) = local_db.insert_simulation.call_args.args + assert simulation.alias == "simulation-alias" + assert len(simulation.inputs) == 1 + assert len(simulation.outputs) == 1 + + +def test_ingest_alias_option_overrides_the_manifest(invoke, local_db, manifest_file): + result = invoke("simulation", "ingest", str(manifest_file), "--alias=override") + + assert result.exit_code == 0 + assert "ALIAS: override" in result.output + (simulation,) = local_db.insert_simulation.call_args.args + assert simulation.alias == "override" + + +def test_ingest_rejects_an_alias_with_reserved_characters( + invoke, local_db, manifest_file +): + """The manifest refuses the alias before the simulation is built. + + ``simulation_ingest`` also has a "warning: alias contains reserved + characters" branch, but ``Manifest.validate_alias`` applies the identical + ``urllib.parse.quote`` check to both the manifest alias and the ``--alias`` + override, so that branch cannot be reached. + """ + result = invoke("simulation", "ingest", str(manifest_file), "--alias=with space") + + assert result.exit_code != 0 + assert "illegal characters in alias" in str(result.exception) + assert not local_db.insert_simulation.called + + +def test_ingest_requires_an_existing_manifest(invoke, local_db, tmp_path): + result = invoke("simulation", "ingest", str(tmp_path / "missing.yaml")) + + assert result.exit_code == 2 + assert not local_db.insert_simulation.called + + +# --------------------------------------------------------------------------- +# simulation query +# --------------------------------------------------------------------------- + + +def test_query_parses_constraints_and_prints_matches(invoke, local_db): + local_db.query_meta.return_value = [make_simulation("match")] + + result = invoke("simulation", "query", "pulse=gt:1000", "run=0") + + assert result.exit_code == 0 + assert "match" in result.output + (constraints,) = local_db.query_meta.call_args.args + assert [(name, value) for name, value, _ in constraints] == [ + ("pulse", "1000"), + ("run", "0"), + ] + + +def test_query_requires_a_constraint(invoke, local_db): + result = invoke("simulation", "query") + + assert result.exit_code != 0 + assert "At least one constraint must be provided" in result.output + assert not local_db.query_meta.called + + +def test_query_rejects_a_constraint_without_a_value(invoke, local_db): + result = invoke("simulation", "query", "pulse") + + assert result.exit_code != 0 + assert "Invalid constraint pulse" in result.output + assert not local_db.query_meta.called + + +# --------------------------------------------------------------------------- +# simulation push +# --------------------------------------------------------------------------- + + +def test_push_validates_and_uploads_the_simulation(invoke, local_db, remote_api): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "push", "sim") + + assert result.exit_code == 0 + assert "Successfully pushed simulation" in result.output + assert remote_api.push_simulation.call_args.args == (simulation,) + assert remote_api.push_simulation.call_args.kwargs["add_watcher"] is False + + +def test_push_records_the_replaced_simulation(invoke, local_db, remote_api): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "push", "sim", "--replaces=older") + + assert result.exit_code == 0 + simulation.set_meta.assert_called_once_with("replaces", "older") + + +def test_push_adds_a_watcher_on_request(invoke, local_db, remote_api): + local_db.get_simulation.return_value = make_simulation("sim") + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "push", "sim", "--add-watcher") + + assert result.exit_code == 0 + assert remote_api.push_simulation.call_args.kwargs["add_watcher"] is True + + +def test_push_fails_when_the_simulation_is_unknown(invoke, local_db, remote_api): + local_db.get_simulation.return_value = None + + result = invoke("simulation", "push", "sim") + + assert result.exit_code != 0 + assert "Failed to find simulation: sim" in result.output + assert not remote_api.push_simulation.called + + +def test_push_reports_validation_failures_without_uploading( + invoke, local_db, remote_api +): + local_db.get_simulation.return_value = make_simulation("sim") + remote_api.get_validation_schemas.return_value = [{"alias": {"type": "string"}}] + + with mock.patch("simdb.cli.commands.simulation.Validator") as validator: + validator.return_value.validate.side_effect = ValidationError("bad metadata") + result = invoke("simulation", "push", "sim") + + assert result.exit_code != 0 + assert "Simulation does not validate: bad metadata" in result.output + assert not remote_api.push_simulation.called + + +# --------------------------------------------------------------------------- +# simulation pull +# --------------------------------------------------------------------------- + + +def test_pull_stores_the_simulation_locally(invoke, local_db, remote_api, tmp_path): + local_db.get_simulation.side_effect = DatabaseError("not found") + pulled = make_simulation("pulled") + remote_api.pull_simulation.return_value = pulled + + result = invoke("simulation", "pull", "sim", str(tmp_path / "out")) + + assert result.exit_code == 0 + assert "Successfully pulled simulation" in result.output + local_db.insert_simulation.assert_called_once_with(pulled) + + +def test_pull_refuses_to_overwrite_an_existing_simulation( + invoke, local_db, remote_api, tmp_path +): + local_db.get_simulation.return_value = make_simulation("sim") + + result = invoke("simulation", "pull", "sim", str(tmp_path / "out")) + + assert result.exit_code != 0 + assert "already exists" in result.output + assert not remote_api.pull_simulation.called + assert not local_db.insert_simulation.called + + +def test_pull_reports_remote_errors(invoke, local_db, remote_api, tmp_path): + local_db.get_simulation.side_effect = DatabaseError("not found") + remote_api.pull_simulation.side_effect = RemoteError("remote is down") + + result = invoke("simulation", "pull", "sim", str(tmp_path / "out")) + + assert result.exit_code != 0 + assert "remote is down" in result.output + assert not local_db.insert_simulation.called + + +# --------------------------------------------------------------------------- +# simulation data +# --------------------------------------------------------------------------- + + +def _data_response(field_data, coordinates=()): + return ImasDataResponse( + simulation="abcd1234", + path="core_profiles/profiles_1d[0]/grid/rho_tor_norm", + occurrence=0, + field=QuantityData(name="grid/rho_tor_norm", units="-", data=field_data), + coordinates=list(coordinates), + ) + + +def test_data_plots_a_one_dimensional_field(invoke, remote_api): + remote_api.get_simulation_data.return_value = _data_response([0.0, 0.5, 1.0]) + + result = invoke("simulation", "data", "sim", "core_profiles/grid/rho_tor_norm") + + assert result.exit_code == 0 + assert "abcd1234" in result.output + assert "occurrence 0" in result.output + assert remote_api.get_simulation_data.call_args.args == ( + "sim", + "core_profiles/grid/rho_tor_norm", + ) + assert remote_api.get_simulation_data.call_args.kwargs == {"dd_version": None} + + +def test_data_forwards_the_requested_dd_version(invoke, remote_api): + remote_api.get_simulation_data.return_value = _data_response(1.5) + + result = invoke( + "simulation", "data", "sim", "core_profiles/time", "--dd-version=4.1.1" + ) + + assert result.exit_code == 0 + assert remote_api.get_simulation_data.call_args.kwargs == {"dd_version": "4.1.1"} + + +def test_data_reports_remote_failures_as_a_clean_error(invoke, remote_api): + remote_api.get_simulation_data.side_effect = RemoteError("no such field") + + result = invoke("simulation", "data", "sim", "core_profiles/nope") + + assert result.exit_code != 0 + assert "no such field" in result.output + + +# --------------------------------------------------------------------------- +# simulation validate +# --------------------------------------------------------------------------- + + +def test_validate_checks_metadata_and_file_checksums(invoke, local_db, remote_api): + file = mock.Mock(uri="file:///data", checksum="abc") + file.generate_checksum.return_value = "abc" + simulation = make_simulation("sim") + simulation.inputs = [file] + simulation.outputs = [] + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "validate", "sim") + + assert result.exit_code == 0 + assert "validation successful" in result.output + + +def test_validate_fails_on_a_checksum_mismatch(invoke, local_db, remote_api): + file = mock.Mock(uri="file:///data", checksum="abc") + file.generate_checksum.return_value = "different" + simulation = make_simulation("sim") + simulation.inputs = [file] + simulation.outputs = [] + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "validate", "sim") + + assert result.exit_code != 0 + assert isinstance(result.exception, ValidationError) + assert "file:///data" in str(result.exception) diff --git a/tests/cli/test_cli_validators.py b/tests/cli/test_cli_validators.py new file mode 100644 index 00000000..614516e4 --- /dev/null +++ b/tests/cli/test_cli_validators.py @@ -0,0 +1,28 @@ +"""Tests for the shared click parameter validators.""" + +import click +import pytest + +from simdb.cli.commands.validators import validate_non_negative, validate_positive + + +@pytest.mark.parametrize("value", [0, 1, 100]) +def test_non_negative_accepts_zero_and_above(value): + assert validate_non_negative(None, None, value) == value + + +@pytest.mark.parametrize("value", [-1, -100]) +def test_non_negative_rejects_negative_values(value): + with pytest.raises(click.BadParameter, match="must be non-negative"): + validate_non_negative(None, None, value) + + +@pytest.mark.parametrize("value", [1, 100]) +def test_positive_accepts_values_above_zero(value): + assert validate_positive(None, None, value) == value + + +@pytest.mark.parametrize("value", [0, -1]) +def test_positive_rejects_zero_and_below(value): + with pytest.raises(click.BadParameter, match="must be greater than zero"): + validate_positive(None, None, value) diff --git a/tests/cli/test_remote_api_helpers.py b/tests/cli/test_remote_api_helpers.py new file mode 100644 index 00000000..73c76cae --- /dev/null +++ b/tests/cli/test_remote_api_helpers.py @@ -0,0 +1,42 @@ +"""Tests for the IDS metadata helper used when pushing IMAS data.""" + +import pytest + +from simdb.cli.remote_api import _meta_list + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + # The list form written since #119. + (["core_profiles", "equilibrium"], ["core_profiles", "equilibrium"]), + (("core_profiles", "equilibrium"), ["core_profiles", "equilibrium"]), + # The display string written by SimDB <= 1.2, which reaches the client + # from a remote that has not been migrated. + ("[core_profiles, equilibrium]", ["core_profiles", "equilibrium"]), + ("core_profiles", ["core_profiles"]), + # Nothing to filter on. + (None, []), + ("", []), + ("[]", []), + ], +) +def test_meta_list_normalises_both_stored_forms(value, expected): + assert _meta_list(value) == expected + + +@pytest.mark.parametrize( + "value", [["core_profiles", "equilibrium"], "[core_profiles, equilibrium]"] +) +def test_ids_names_match_however_the_value_was_stored(value): + """push_simulation skips any IDS whose name is not in this list. + + Wrapping the display string instead of splitting it yields + ``["[core_profiles, equilibrium]"]``, which matches no IDS name at all and so + silently skips every file of the simulation. + """ + ids_list = _meta_list(value) + + assert "core_profiles" in ids_list + assert "equilibrium" in ids_list + assert "core_sources" not in ids_list diff --git a/tests/cli/utils.py b/tests/cli/utils.py deleted file mode 100644 index 7c99c36f..00000000 --- a/tests/cli/utils.py +++ /dev/null @@ -1,51 +0,0 @@ -from pathlib import Path - - -def config_test_file() -> Path: - config = """\ -[remote "test"] -url = http://0.0.0.0:5000/ -default = True -token = 123ABC -""" - config_file = Path(__file__).parent / "test.cfg" - config_file.write_text(config) - return config_file - - -def create_manifest() -> Path: - manifest = """\ -manifest_version: 2 -alias: simulation-alias - -# Data and configuration files -inputs: -# - uri: simdb://simdb.iter.org/123e4567-e89b-12d3-a456-426655440000 - - uri: file:///$MANIFEST_DIR/utils.py - - uri: imas:///user?shot=10000&run=0&database=west -# - uri: imas+uda:///TOKAMAK?shot=10000&run=0&server=uda.server.org:56565 - -# Data and log files. -outputs: - - uri: file:///$MANIFEST_DIR/utils.py - - uri: imas:///user?shot=10000&run=1&database=west - -metadata: -- values: - workflow: - name: Workflow Name - git: ssh://git@git.iter.org/wf/workflow.git - branch: master - commit: 079e84d5ae8a0eec6dcf3819c98f3c05f48e952f - codes: - - Code 1: - git: ssh://git@git.iter.org/eq/code.git - commit: 079e84d5ae8a0eec6dcf3819c98f3c05f48e952f -""" - manifest_file = Path(__file__).parent / "manifest.yaml" - manifest_file.write_text(manifest) - return manifest_file - - -def get_file_path(file_name) -> Path: - return Path(__file__).parent / file_name From a974a8d4336d2c4aef14abcd0226d5d7191b8ed3 Mon Sep 17 00:00:00 2001 From: Yannick de Jong Date: Wed, 26 Aug 2026 10:57:51 +0200 Subject: [PATCH 2/2] Fix default remote handling when using options --- src/simdb/cli/commands/simulation.py | 29 +++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/simdb/cli/commands/simulation.py b/src/simdb/cli/commands/simulation.py index 551dde77..c6f523b3 100644 --- a/src/simdb/cli/commands/simulation.py +++ b/src/simdb/cli/commands/simulation.py @@ -3,7 +3,7 @@ import urllib.parse from itertools import chain from pathlib import Path -from typing import Any, List, Optional, Tuple, Type +from typing import Any, List, Optional, Tuple import appdirs import click @@ -200,19 +200,22 @@ def simulation_ingest(config: Config, manifest_file: str, alias: str): click.echo("ALIAS: " + simulation.alias + "\nUUID: " + str(simulation.uuid)) -def n_required_args_adaptor(n) -> Type[click.Command]: - class NRequiredArgs(click.Command): - NArgs = n +class OptionalRemoteCommand(click.Command): + """A command declared as `[REMOTE] ARG...` whose REMOTE may be left out.""" - def parse_args(self, ctx, args): - if len(args) == self.NArgs: - args.insert(0, "") - super().parse_args(ctx, args) + def parse_args(self, ctx, args): + arguments = [p for p in self.get_params(ctx) if isinstance(p, click.Argument)] + if self._count_values_given(ctx, args, arguments) < len(arguments): + args = ["", *args] + super().parse_args(ctx, args) - return NRequiredArgs + def _count_values_given(self, ctx, args, arguments) -> int: + """Count how many of the ARGUMENTS the command line provides a value for.""" + values = self.make_parser(ctx).parse_args(list(args))[0] + return sum(1 for argument in arguments if values.get(argument.name) is not None) -@simulation.command("push", cls=n_required_args_adaptor(1)) +@simulation.command("push", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id") @@ -257,7 +260,7 @@ def simulation_push( click.echo(f"Successfully pushed simulation {simulation.uuid}") -@simulation.command("pull", cls=n_required_args_adaptor(2)) +@simulation.command("pull", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id") @@ -380,7 +383,7 @@ def simulation_query( ) -@simulation.command("data", cls=n_required_args_adaptor(2)) +@simulation.command("data", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id") @@ -447,7 +450,7 @@ def simulation_data( print_quantity(coord, label=f"coord {coord.name}", show_stats=False) -@simulation.command("validate", cls=n_required_args_adaptor(1)) +@simulation.command("validate", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id")