From 65d1bd9521bc5c93ff0cdf76d4aac2bc7fe79b32 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Tue, 22 Sep 2026 12:57:38 -0600 Subject: [PATCH 1/4] tests(rimport): red test for where the emptiness warning prints Fails today: the "no files found" warning is logged inside the walk loop, so it prints above the expansion count, while the skip warning from the same function prints below it. Pins the whole order below the count -- count, then empties, then skips -- and the indent that marks both warnings as subordinate to the count. Co-Authored-By: Claude Opus 5 (1M context) --- tests/rimport/test_expand_directories.py | 26 ++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/rimport/test_expand_directories.py b/tests/rimport/test_expand_directories.py index 7bb0029..12bce12 100644 --- a/tests/rimport/test_expand_directories.py +++ b/tests/rimport/test_expand_directories.py @@ -7,6 +7,8 @@ import importlib.util from importlib.machinery import SourceFileLoader +from shared import INDENT + # Import rimport module from file without .py extension rimport_path = os.path.join( @@ -276,6 +278,30 @@ def test_expansion_count_is_logged_before_any_skip_warning(tmp_path, caplog): assert caplog.text.index("expanded 1 director(ies)") < caplog.text.index("skipping") +def test_expansion_count_is_logged_before_any_emptiness_warning(tmp_path, caplog): + """The count line is the blast radius, and it belongs at the top where it cannot be + pushed down the screen by one warning per empty directory named. Both warnings this + function emits sit below it, and both are indented to say so.""" + empty = tmp_path / "empty" + empty.mkdir() + d = tmp_path / "d" + d.mkdir() + (d / "a.nc").write_text("a") + locked = d / "locked" + locked.mkdir() + os.chmod(locked, 0o000) + + try: + with caplog.at_level(logging.INFO, logger="rimport_relink"): + rimport.expand_directories([empty, d], tmp_path) + finally: + os.chmod(locked, 0o700) + + assert caplog.text.index("expanded 2 director(ies)") < caplog.text.index("no files found") + assert caplog.text.index("no files found") < caplog.text.index("skipping") + assert f"{INDENT}rimport: no files found under {empty}" in caplog.text + + def test_directory_named_twice_is_reported_once(tmp_path, caplog): """Naming the same directory twice is one directory, not two, so the blast-radius line counts it once.""" From 20370b089570c1aa95c13ab5bf9a39bc45a6603c Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Tue, 22 Sep 2026 12:58:48 -0600 Subject: [PATCH 2/4] rimport: Log "no files found" below the expansion count, indented expand_directories() collects the empty directories during the walk and reports them where the skip warnings are already reported, after the count. Both warnings now sit below the count and are indented under it; the emptiness warning's text is otherwise unchanged. Presentation only. Entries, skips, counts and exit codes are untouched. Verified: full pytest suite green, and the reproduction from the issue report run by hand. Resolves ESMCI/inputdataTools#33. Co-Authored-By: Claude Opus 5 (1M context) --- rimport | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/rimport b/rimport index 29fb9e4..1d7dcba 100755 --- a/rimport +++ b/rimport @@ -311,6 +311,7 @@ def expand_directories( named_paths = set(paths) named_by_path: dict[Path, bool] = {} skips: List[Skip] = [] + empty: List[Path] = [] n_dirs = 0 expanded_files: set[Path] = set() @@ -335,7 +336,10 @@ def expand_directories( found, walk_skips = walk_files(path) skips.extend(walk_skips) if not found and not walk_skips: - logger.warning("rimport: no files found under %s", path) + # Collected, not reported: every warning this function emits is logged + # below the count. A walk that returned nothing because it could not be + # read is not an empty directory, and is reported as a skip instead. + empty.append(path) for found_path in found: expanded_files.add(found_path) named_by_path.setdefault(found_path, False) @@ -351,11 +355,17 @@ def expand_directories( deduped.setdefault(skip.path, Skip(skip.path, skip.reason, skip.path in named_paths)) skips = list(deduped.values()) + # Everything below is logged after the count, and indented under it. The count is the + # blast radius -- the one line worth reading before a large run is left to finish -- so + # it stays at the top of the output however many directories turn out to warn. if n_dirs: logger.info( "rimport: expanded %d director(ies) to %d file(s)", n_dirs, len(expanded_files) ) + for path in empty: + logger.warning("%srimport: no files found under %s", INDENT, path) + for skip in skips: # Report a DISCOVERED skip here, during expansion, where the run reached it; the # end-of-run summary repeats it. A skip the user NAMED is not reported here at all: From c5fd56178702074998644fa6cfa8710bda5f1eb0 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Tue, 22 Sep 2026 13:05:54 -0600 Subject: [PATCH 3/4] tests(rimport): Pin the skip warning's indent too The ordering test's docstring said both warnings below the count are indented, but only the emptiness half was asserted; dropping INDENT from the skip warning left the suite green. Co-Authored-By: Claude Opus 5 (1M context) --- tests/rimport/test_expand_directories.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/rimport/test_expand_directories.py b/tests/rimport/test_expand_directories.py index 12bce12..8339ecf 100644 --- a/tests/rimport/test_expand_directories.py +++ b/tests/rimport/test_expand_directories.py @@ -300,6 +300,7 @@ def test_expansion_count_is_logged_before_any_emptiness_warning(tmp_path, caplog assert caplog.text.index("expanded 2 director(ies)") < caplog.text.index("no files found") assert caplog.text.index("no files found") < caplog.text.index("skipping") assert f"{INDENT}rimport: no files found under {empty}" in caplog.text + assert f"{INDENT}rimport: skipping '{locked}'" in caplog.text def test_directory_named_twice_is_reported_once(tmp_path, caplog): From 3ae1f9cf955ba4eaf037a3fd64ebd1e0ffe85f69 Mon Sep 17 00:00:00 2001 From: Sam Rabin Date: Tue, 22 Sep 2026 13:29:06 -0600 Subject: [PATCH 4/4] tests(rimport): Pin one emptiness warning per duplicated argument The count line and the skips were already pinned against a directory named twice; the emptiness warning was not, and both existing duplicate tests use a non-empty directory, so removing the dedup left it uncaught. Co-Authored-By: Claude Opus 5 (1M context) --- tests/rimport/test_expand_directories.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/rimport/test_expand_directories.py b/tests/rimport/test_expand_directories.py index 8339ecf..93f5168 100644 --- a/tests/rimport/test_expand_directories.py +++ b/tests/rimport/test_expand_directories.py @@ -316,6 +316,19 @@ def test_directory_named_twice_is_reported_once(tmp_path, caplog): assert "expanded 1 director(ies) to 1 file(s)" in caplog.text +def test_empty_directory_named_twice_warns_once(tmp_path, caplog): + """One empty directory is one warning, however many of the named arguments reach it. + The count line and the skips are already pinned against duplicate arguments; without + this the emptiness warning is the one report that could double.""" + d = tmp_path / "empty" + d.mkdir() + + with caplog.at_level(logging.WARNING, logger="rimport_relink"): + rimport.expand_directories([d, d], tmp_path) + + assert caplog.text.count("no files found") == 1 + + def test_duplicate_arguments_do_not_duplicate_a_skip(tmp_path, caplog): """One unreadable directory is one skip, however many of the named arguments reach it. Otherwise it is warned about twice, listed twice in the end-of-run summary, and counted