Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog_entries/749.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed the "file already exists" errors from `save_report` and the CLI `--json-file-path` option telling the user to pass a `-o` flag, which does not exist; they now name `--overwrite`.
4 changes: 3 additions & 1 deletion src/nwbinspector/_formatting.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,9 @@ def save_report(report_file_path: Union[str, Path], formatted_messages: list[str
report_file_path = Path(report_file_path)

if report_file_path.exists() and not overwrite:
raise FileExistsError(f"The file {report_file_path} already exists! Set 'overwrite=True' or pass '-o' flag.")
raise FileExistsError(
f"The file {report_file_path} already exists! Set 'overwrite=True' or pass the '--overwrite' flag."
)

with open(file=report_file_path, mode="w", newline="\n") as file:
for line in formatted_messages:
Expand Down
10 changes: 8 additions & 2 deletions src/nwbinspector/_nwbinspector_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,14 @@ def _nwbinspector_cli(
show_progress_bar = True if progress_bar is None else strtobool(progress_bar)
handled_modules = [] if modules is None else modules.split(",")

# Refuse an existing output file before the inspection runs, so the user does not learn of it only after a
# whole folder or dandiset has been scanned
for output_file_path in (json_file_path, report_file_path):
if output_file_path is not None and Path(output_file_path).exists() and not overwrite:
raise FileExistsError(
f"The file {output_file_path} already exists! Pass the '--overwrite' flag to overwrite."
)

# Trigger the import of custom checks that have been registered and exposed to their respective modules
for module in handled_modules:
importlib.import_module(name=module)
Expand Down Expand Up @@ -214,8 +222,6 @@ def _nwbinspector_cli(
raise SystemExit(1)

if json_file_path is not None:
if Path(json_file_path).exists() and not overwrite:
raise FileExistsError(f"The file {json_file_path} already exists! Specify the '-o' flag to overwrite.")
with open(file=json_file_path, mode="w") as fp:
json_report = dict(header=_get_report_header(), messages=messages)
json.dump(obj=json_report, fp=fp, cls=InspectorOutputJSONEncoder)
Expand Down
71 changes: 71 additions & 0 deletions tests/test_formatting.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,74 @@ def test_format_messages_with_issues(self):

self.assertIn("Scanned 4 file(s).", formatted_messages)
self.assertIn("Found 3 issues across 2 file(s):", formatted_messages)


def test_save_report_existing_file_message_names_overwrite_flag(tmp_path):
"""The message used to point at a '-o' flag that the CLI does not have."""
import pytest

from nwbinspector import save_report

report_file_path = tmp_path / "report.txt"
report_file_path.write_text("existing")

with pytest.raises(FileExistsError, match="--overwrite"):
save_report(report_file_path=report_file_path, formatted_messages=[], overwrite=False)


def test_cli_existing_json_file_message_names_overwrite_flag(tmp_path):
"""The existence check runs before the inspection, so no NWB file is needed to reach it."""
from click.testing import CliRunner

from nwbinspector._nwbinspector_cli import _nwbinspector_cli

json_file_path = tmp_path / "report.json"
json_file_path.write_text("{}")

result = CliRunner().invoke(_nwbinspector_cli, [str(tmp_path), "--json-file-path", str(json_file_path)])

assert isinstance(result.exception, FileExistsError)
assert "--overwrite" in str(result.exception)
assert "-o'" not in str(result.exception)


def test_cli_existing_report_file_message_names_overwrite_flag(tmp_path):
from click.testing import CliRunner

from nwbinspector._nwbinspector_cli import _nwbinspector_cli

report_file_path = tmp_path / "report.txt"
report_file_path.write_text("existing")

result = CliRunner().invoke(_nwbinspector_cli, [str(tmp_path), "--report-file-path", str(report_file_path)])

assert isinstance(result.exception, FileExistsError)
assert "--overwrite" in str(result.exception)


def test_cli_overwrite_flag_allows_existing_output_files(tmp_path):
"""With --overwrite the up-front check passes and both files are written, here for an empty folder."""
from click.testing import CliRunner

from nwbinspector._nwbinspector_cli import _nwbinspector_cli

json_file_path = tmp_path / "report.json"
json_file_path.write_text("{}")
report_file_path = tmp_path / "report.txt"
report_file_path.write_text("existing")

result = CliRunner().invoke(
_nwbinspector_cli,
[
str(tmp_path),
"--json-file-path",
str(json_file_path),
"--report-file-path",
str(report_file_path),
"--overwrite",
],
)

assert result.exception is None, result.output
assert json_file_path.read_text() != "{}"
assert report_file_path.read_text() != "existing"
Loading