diff --git a/CHANGELOG.md b/CHANGELOG.md index cdd7181..1863bfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,42 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Monitor reported 0.0% CPU for every child process.** psutil records the + CPU-times baseline on the `Process` *instance*, and `children(recursive=True)` + builds fresh objects on every sample, so each descendant's first (always-zero) + reading was the only one ever taken. Process instances are now cached by PID, + so baselines persist. This mattered whenever the real work ran in a child + rather than the launched process — a Windows venv `python.exe` shim, or + EddyPro spawning workers — which produced exactly the all-zero CPU column the + process-tree fix was meant to eliminate. + +- **`eddypro-batch` with no subcommand printed a config error instead of + help.** `main()` validated the config file before dispatching, so on a fresh + clone -- where `config/config.yaml` does not exist yet -- the first command + anyone runs failed with "Configuration file not found" rather than showing + usage. The no-command case now short-circuits to `print_help()`. + +- **Pre-v2 metrics files are now reported as `UNKNOWN`** instead of being read + as a run with no bottleneck. Those files lack every process-tree column, so + the analyser was summarising absent data as `CPU 0.0%` and concluding there + was headroom to spare. + +### Changed + +- **`config/config.yaml` is no longer tracked in git.** It holds + machine-specific absolute paths that churned in every diff. The file stays + where it is and the default `--config` path is unchanged; only + `config/config.yaml.example` is version-controlled now, so a fresh clone must + copy it before first use. + +- **Default `performance_thresholds` disk limits raised to SATA SSD scale** + (`disk_high_mb_per_s` 100 → 450, `disk_moderate_mb_per_s` 50 → 250, + `disk_high_iops` 1000 → 20000). The previous values assumed a mechanical disk + and flagged ordinary SSD runs as disk-bound. `docs/CONFIG.md` now carries a + per-medium table for NVMe, SATA, and mechanical drives. + ### Added - **`--version` global CLI flag** to print the installed package version and exit. diff --git a/CLAUDE.md b/CLAUDE.md index e9d6707..4b90c41 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -51,7 +51,9 @@ pre-commit run --all-files `D:/L0_raw/`, or `D:/L1_processed/`. These hold irreplaceable raw and processed field data. - `config/config.yaml` is the user's live working config with - machine-specific paths. Don't commit changes to it. + machine-specific paths. It is untracked and gitignored — never re-add it + with `git add -f`. Edit `config/config.yaml.example` instead when a change + belongs in the repo. ## Code quality diff --git a/README.md b/README.md index a62cf96..edbac21 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,9 @@ cp config/config.yaml.example config/config.yaml # Edit config/config.yaml with your paths and settings ``` +This step is required: `config/config.yaml` is gitignored (it holds +machine-specific paths), so a fresh clone does not contain one. + **Key configuration items:** - Set `eddypro_executable` to your EddyPro installation path diff --git a/config/config.yaml b/config/config.yaml deleted file mode 100644 index 1c00527..0000000 --- a/config/config.yaml +++ /dev/null @@ -1,85 +0,0 @@ -# Configuration for EddyPro processing job - -# Specify the path to the EddyPro executable -eddypro_executable: "C:/Program Files/LI-COR/EddyPro-7.0.9/bin/eddypro_rp.exe" - -# Specify the site ID you want to process -site_id: GL-ZaF -#site_id: GL-ZaH -#site_id: GL-Dsk -#site_id: GL-NuF -#site_id: GL-NuH - -# List of years you want to process for this site -years_to_process: - - #- 2000 - #- 2001 - #- 2002 - #- 2003 - #- 2004 - #- 2005 - #- 2016 - #- 2007 - #- 2008 - #- 2009 - #- 2010 - #- 2011 - #- 2012 - #- 2013 - #- 2014 - #- 2015 - #- 2016 - #- 2017 - #- 2018 - #- 2019 - #- 2020 - #- 2021 - #- 2022 - #- 2023 - #- 2024 - - 2025 -# Input directory pattern for each year and site -# Use `{year}` and `{site_id}` as placeholders -#input_dir_pattern: "C:/Users/au710242/Code/Python/eddypro_batch_processor/data/raw/{site_id}/{year}" - -input_dir_pattern: "D:/L0_raw/{site_id}/{year}/ec/rflux_csv" - -# Output directory pattern for each year and site -# Use `{year}` and `{site_id}` as placeholders -#output_dir_pattern: "C:/Users/au710242/Code/Python/eddypro_batch_processor/data/processed/{site_id}/{year}" - -output_dir_pattern: "D:/L1_processed/{site_id}/{year}/ec_rflux" - -# Path to the ECMD CSV file -#ecmd_file: "C:/Users/au710242/Code/Python/eddypro_batch_processor/data/GL-ZaF_ecmd.csv" -ecmd_file: "D:/L1_processed/{site_id}/ecmd/{site_id}_ecmd.csv" - -# Enable or disable multiprocessing -multiprocessing: False # Set to False to disable multiprocessing - -# Maximum number of CPU cores to use for multiprocessing -max_processes: 16 # Adjust this number based on your requirements - -# Control output streaming (EddyPro subprocess outputs) -stream_output: True # Set to False to keep EddyPro output quiet - -# Control logging verbosity -log_level: INFO # Options: DEBUG, INFO, WARNING, ERROR, CRITICAL - -# Optional log file path (null disables file logging) -log_file: logs/eddypro_processing.log - -# Log rotation (bytes) and backup count (0 disables rotation) -log_max_bytes: 10485760 # 10 MB -log_backup_count: 5 - -# Capture EddyPro stdout/stderr in logs -log_eddypro_output: True - -# Performance monitoring configuration -metrics_interval_seconds: 0.5 # Sampling interval for performance monitoring - -# Reporting configuration -reports_dir: null # Optional: Custom reports directory (default: {output_dir}/reports) -report_charts: plotly # Chart engine: plotly, svg, or none (default: plotly) diff --git a/config/config.yaml.example b/config/config.yaml.example index 20e8bdc..87ae8be 100644 --- a/config/config.yaml.example +++ b/config/config.yaml.example @@ -59,12 +59,19 @@ monitoring_enabled: true metrics_interval_seconds: 0.5 # Optional: tune how the bottleneck analyser classifies a run. Defaults assume a -# mechanical disk; raise the disk limits substantially for NVMe. See -# docs/CONFIG.md for the full table of keys and defaults. +# SATA SSD (~550 MB/s). Set these to match the drive the input data lives on, +# otherwise the disk verdict is meaningless. See docs/CONFIG.md for every key. #performance_thresholds: # cpu_high_percent: 90 # memory_high_percent: 85 -# disk_high_mb_per_s: 100 +# # NVMe SSD: +# disk_high_mb_per_s: 3000 +# disk_moderate_mb_per_s: 1500 +# disk_high_iops: 200000 +# # Mechanical disk: +# #disk_high_mb_per_s: 150 +# #disk_moderate_mb_per_s: 80 +# #disk_high_iops: 150 # Reporting configuration reports_dir: null # Optional: Custom reports directory (default: {output_dir}/reports) diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 8ba4539..1ffde30 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -8,6 +8,16 @@ Default location: `config/config.yaml` Override with: `--config /path/to/config.yaml` +`config/config.yaml` is **not tracked in git** — it holds machine-specific +absolute paths that differ per install. It is covered by `.gitignore`, so a +fresh clone has no `config/config.yaml` until you create one: + +```powershell +cp config/config.yaml.example config/config.yaml +``` + +Only `config/config.yaml.example` is version-controlled. + ## Configuration Schema ### Complete Example @@ -408,8 +418,11 @@ monitoring_enabled: false **Default:** see table below **Description:** Tunes how the bottleneck analyser classifies a run. The -defaults assume a mechanical disk; on NVMe storage the disk limits should be -raised substantially or every run will be reported as disk-bound. +defaults assume a **SATA SSD** (~550 MB/s sequential), which is the common case +for the bulk storage EddyPro reads from. The disk limits must match the drive +the input data actually lives on: leave NVMe limits at the SATA default and +ordinary runs are reported as disk-bound; leave mechanical-disk limits at the +SATA default and a genuinely saturated disk is never flagged at all. | Key | Default | Meaning | |-----|---------|---------| @@ -418,17 +431,33 @@ raised substantially or every run will be reported as disk-bound. | `cpu_idle_percent` | 40 | Below this, a busy disk is read as the limiting factor | | `memory_high_percent` | 85 | System memory use that counts as RED | | `memory_moderate_percent` | 70 | System memory use that counts as YELLOW | -| `disk_high_mb_per_s` | 100 | Combined read+write throughput counting as RED | -| `disk_moderate_mb_per_s` | 50 | Throughput counting as YELLOW | -| `disk_high_iops` | 1000 | Combined IOPS above which latency is the suspect | +| `disk_high_mb_per_s` | 450 | Combined read+write throughput counting as RED | +| `disk_moderate_mb_per_s` | 250 | Throughput counting as YELLOW | +| `disk_high_iops` | 20000 | Combined IOPS above which latency is the suspect | Unknown keys are ignored, so a config written for a newer version still loads. +Suggested disk limits by medium: + +| Medium | `disk_high_mb_per_s` | `disk_moderate_mb_per_s` | `disk_high_iops` | +|--------|---------------------|--------------------------|------------------| +| NVMe SSD | 3000 | 1500 | 200000 | +| SATA SSD (default) | 450 | 250 | 20000 | +| Mechanical / USB HDD | 150 | 80 | 150 | + +On Windows, check which you have with: + +```powershell +Get-PhysicalDisk | Select-Object FriendlyName, MediaType, BusType +Get-Partition | Where-Object DriveLetter | Select-Object DriveLetter, DiskNumber +``` + **Example (NVMe):** ```yaml performance_thresholds: - disk_high_mb_per_s: 2000 - disk_moderate_mb_per_s: 1000 + disk_high_mb_per_s: 3000 + disk_moderate_mb_per_s: 1500 + disk_high_iops: 200000 ``` **CLI Override:** none — config only. diff --git a/examples/multi_year_config.yaml b/examples/multi_year_config.yaml index b972b70..aca94c7 100644 --- a/examples/multi_year_config.yaml +++ b/examples/multi_year_config.yaml @@ -44,6 +44,13 @@ log_eddypro_output: true # Performance monitoring: sample CPU/memory/disk every second per year-worker. # Set monitoring_enabled: false for maximum throughput on large batches. monitoring_enabled: true + +# Bottleneck thresholds must match the drive the data lives on. These are the +# defaults, sized for a SATA SSD; see docs/CONFIG.md for NVMe and HDD values. +performance_thresholds: + disk_high_mb_per_s: 450 + disk_moderate_mb_per_s: 250 + disk_high_iops: 20000 metrics_interval_seconds: 1.0 # Reports directory set explicitly: it otherwise defaults to the FIRST diff --git a/src/eddypro_batch_processor/analysis.py b/src/eddypro_batch_processor/analysis.py index d3fb400..a2ce91f 100644 --- a/src/eddypro_batch_processor/analysis.py +++ b/src/eddypro_batch_processor/analysis.py @@ -46,13 +46,24 @@ class PerformanceThresholds(TypedDict, total=False): "cpu_idle_percent": 40.0, "memory_high_percent": 85.0, "memory_moderate_percent": 70.0, - # Roughly a mechanical-disk ceiling; raise substantially for NVMe. - "disk_high_mb_per_s": 100.0, - "disk_moderate_mb_per_s": 50.0, - "disk_high_iops": 1000.0, + # Calibrated for a SATA SSD (~550 MB/s sequential), which is the common + # case for the bulk storage EddyPro reads from. Adjust for other media: + # NVMe SSD -> disk_high ~3000, moderate ~1500, iops ~200000 + # Mechanical -> disk_high ~150, moderate ~80, iops ~150 + "disk_high_mb_per_s": 450.0, + "disk_moderate_mb_per_s": 250.0, + "disk_high_iops": 20000.0, } +# Columns the process-tree monitor must emit. Their absence means the file was +# written by the pre-v2 monitor, whose figures were all 0.0 anyway (it sampled +# the shell wrapper rather than EddyPro). Reporting UNKNOWN is honest; reporting +# "no bottleneck, CPU 0.0%" from those files is not. +CANONICAL_COLUMNS = ("cpu_percent", "memory_mb", "read_mb", "write_mb") +LEGACY_COLUMNS = ("process_cpu_percent", "process_memory_rss", "process_io_read_bytes") + + @dataclass class MetricStats: """Summary statistics for one metric series.""" @@ -170,6 +181,24 @@ def analyze_rows( self, rows: list[dict[str, Any]], scenario_name: str = "baseline" ) -> ScenarioAnalysis: """Analyse already-parsed metric rows.""" + if rows and not any(c in rows[0] for c in CANONICAL_COLUMNS): + legacy = any(c in rows[0] for c in LEGACY_COLUMNS) + detail = ( + "written by a pre-v2 monitor, whose process metrics were all 0.0" + if legacy + else "missing every process-tree column" + ) + logger.warning( + f"Metrics for {scenario_name} are {detail}; cannot classify." + ) + return ScenarioAnalysis( + scenario_name=scenario_name, + sample_count=len(rows), + explanation=( + f"Unrecognised metrics schema ({detail}). Re-run with the " + f"current version to get a bottleneck verdict." + ), + ) def series(key: str) -> list[float]: return [v for v in (_to_float(r.get(key)) for r in rows) if v is not None] diff --git a/src/eddypro_batch_processor/cli.py b/src/eddypro_batch_processor/cli.py index 4b001e0..78e416b 100644 --- a/src/eddypro_batch_processor/cli.py +++ b/src/eddypro_batch_processor/cli.py @@ -1327,6 +1327,13 @@ def main() -> int: # Set up logging early setup_logging(args.log_level) + # With no subcommand there is nothing to configure, so help must print + # rather than a config error. On a fresh clone config/config.yaml does not + # exist yet, and a bare `eddypro-batch` is the first thing anyone runs. + if not args.command: + parser.print_help() + return 1 + # Validate config file exists if provided if hasattr(args, "config") and args.config: config_path = Path(args.config) @@ -1334,18 +1341,15 @@ def main() -> int: logging.error(f"Configuration file not found: {config_path}") return 1 - # Route to appropriate command handler - if args.command == "run": - return cmd_run(args) - elif args.command == "scenarios": - return cmd_scenarios(args) - elif args.command == "validate": - return cmd_validate(args) - elif args.command == "status": - return cmd_status(args) - else: - parser.print_help() - return 1 + # Route to appropriate command handler. argparse restricts args.command to + # these choices and the empty case returned above, so there is no fallback. + handlers = { + "run": cmd_run, + "scenarios": cmd_scenarios, + "validate": cmd_validate, + "status": cmd_status, + } + return handlers[args.command](args) if __name__ == "__main__": diff --git a/src/eddypro_batch_processor/monitor.py b/src/eddypro_batch_processor/monitor.py index 6c8c149..b03a1a8 100644 --- a/src/eddypro_batch_processor/monitor.py +++ b/src/eddypro_batch_processor/monitor.py @@ -123,6 +123,12 @@ def __init__( self._io_baseline: tuple[float, float, float, float] | None = None self._primed_pids: set[int] = set() + # psutil records the CPU-times baseline on the Process *instance*, so a + # freshly constructed object always reports 0.0. children() builds new + # objects on every call, so the instances must be cached by PID or every + # descendant reads as idle for the whole run. + self._proc_cache: dict[int, psutil.Process] = {} + # Previous sample state, for delta-based rate computation self._prev_time: float | None = None self._prev_io: tuple[float, float, float, float] | None = None @@ -285,10 +291,26 @@ def _iter_tracked(self) -> list[Any]: if not self._process: return [] try: - return [self._process, *self._process.children(recursive=True)] + children = self._process.children(recursive=True) except Exception: return [] + tracked: list[Any] = [self._process] + live_pids = {self._process.pid} + for child in children: + # setdefault keeps the first instance seen for this PID so its CPU + # baseline survives; the newly built object is discarded. + tracked.append(self._proc_cache.setdefault(child.pid, child)) + live_pids.add(child.pid) + + # Drop instances for processes that have exited. Their cumulative I/O is + # already retained separately in _io_by_pid, so nothing is lost. + for pid in self._proc_cache.keys() - live_pids: + del self._proc_cache[pid] + self._primed_pids.discard(pid) + + return tracked + def _collect_sample(self) -> dict[str, Any] | None: """ Collect a single performance sample. diff --git a/tests/test_analysis.py b/tests/test_analysis.py index 8e2767e..e67e707 100644 --- a/tests/test_analysis.py +++ b/tests/test_analysis.py @@ -86,8 +86,8 @@ def test_disk_throughput_bound(self, tmp_path): cpu_percent=8.0, memory_mb=300.0, system_memory_percent=35.0, - read_mb_per_s=180.0, - write_mb_per_s=20.0, + read_mb_per_s=500.0, + write_mb_per_s=100.0, ) result = BottleneckAnalyzer().analyze(csv_path) assert result.primary_bottleneck == "DISK_THROUGHPUT" @@ -103,8 +103,8 @@ def test_disk_iops_bound(self, tmp_path): system_memory_percent=30.0, read_mb_per_s=5.0, write_mb_per_s=2.0, - read_iops=3000.0, - write_iops=500.0, + read_iops=25000.0, + write_iops=5000.0, ) result = BottleneckAnalyzer().analyze(csv_path) assert result.primary_bottleneck == "DISK_IOPS" @@ -142,8 +142,8 @@ def test_cpu_takes_priority_over_disk(self, tmp_path): tmp_path / "metrics.csv", cpu_percent=96.0, system_memory_percent=40.0, - read_mb_per_s=200.0, - write_mb_per_s=200.0, + read_mb_per_s=400.0, + write_mb_per_s=400.0, ) assert BottleneckAnalyzer().analyze(csv_path).primary_bottleneck == "CPU" @@ -156,14 +156,14 @@ def test_custom_disk_threshold_changes_verdict(self, tmp_path): tmp_path / "metrics.csv", cpu_percent=8.0, system_memory_percent=30.0, - read_mb_per_s=180.0, + read_mb_per_s=600.0, ) assert BottleneckAnalyzer().analyze(csv_path).primary_bottleneck == ( "DISK_THROUGHPUT" ) - # On an NVMe drive 180 MB/s is unremarkable, so raise the ceiling. + # On an NVMe drive 600 MB/s is unremarkable, so raise the ceiling. relaxed = BottleneckAnalyzer( - {"disk_high_mb_per_s": 2000.0, "disk_moderate_mb_per_s": 1000.0} + {"disk_high_mb_per_s": 3000.0, "disk_moderate_mb_per_s": 1500.0} ) assert relaxed.analyze(csv_path).primary_bottleneck == "NONE" @@ -202,6 +202,48 @@ def test_blank_and_malformed_cells(self, tmp_path): assert result.cpu.max == 95.0 +class TestLegacySchema: + """Pre-v2 metrics files must be refused, not silently read as all-zero.""" + + LEGACY_FIELDS = [ + "process_cpu_percent", + "process_io_read_bytes", + "process_io_write_bytes", + "process_memory_rss", + "relative_time", + "system_cpu_percent", + "system_memory_percent", + "timestamp", + ] + + def _write_legacy(self, path): + with path.open("w", newline="", encoding="utf-8") as f: + writer = csv.DictWriter(f, fieldnames=self.LEGACY_FIELDS) + writer.writeheader() + for i in range(10): + row = dict.fromkeys(self.LEGACY_FIELDS, 0.0) + row["relative_time"] = float(i) + row["system_cpu_percent"] = 12.0 + row["system_memory_percent"] = 35.0 + writer.writerow(row) + return path + + def test_legacy_file_reports_unknown(self, tmp_path): + path = self._write_legacy(tmp_path / "metrics_rp.csv") + result = BottleneckAnalyzer().analyze(path) + # The old files really do contain 10 rows of zeros; saying "no + # bottleneck, CPU 0.0%" from them would be a confident lie. + assert result.primary_bottleneck == "UNKNOWN" + assert result.sample_count == 10 + assert "pre-v2" in result.explanation + + def test_current_schema_is_not_mistaken_for_legacy(self, tmp_path): + path = make_series( + tmp_path / "metrics.csv", cpu_percent=95.0, system_memory_percent=30.0 + ) + assert BottleneckAnalyzer().analyze(path).primary_bottleneck == "CPU" + + class TestDominantBottleneck: """Aggregation across several analyses.""" @@ -287,8 +329,15 @@ def col(name): # The load is single-threaded, so normalised CPU is small on a many-core # box; cpu_percent_of_core is the column that must show real work. - assert max(col("cpu_percent_of_core")) > 20.0, ( - "process-tree CPU never rose above 20% of a core -- the monitor is " + # + # The failure mode being guarded against produces exactly 0.0 on every + # sample, so the discriminator is "did any real work register at all", + # not a magnitude. A throttled shared CI runner legitimately reports + # well under 20% of a core here, so asserting a specific level would + # only measure the runner. + cpu_of_core = col("cpu_percent_of_core") + assert max(cpu_of_core) > 1.0, ( + "process-tree CPU never registered any work -- the monitor is " "measuring the wrong process again" ) assert max(col("memory_mb")) > 1.0, "process memory looks like a shell" @@ -324,6 +373,69 @@ def test_analysis_of_real_run_is_not_unknown(self, tmp_path, burner_script): assert result.sample_count > 0 assert result.total_read_mb + result.total_write_mb > 0 + def test_cpu_is_measured_for_descendants_not_just_the_root( + self, tmp_path, burner_script + ): + """ + The root must not be the only process whose CPU is counted. + + psutil keeps the CPU-times baseline on the Process *instance*, and + ``children()`` returns freshly built objects on every call. Without an + instance cache each descendant reports 0.0 forever -- which is exactly + what happens when the launched command is a stub that re-execs into a + child (a Windows venv ``python.exe`` shim, or EddyPro spawning workers). + Here the parent deliberately idles so all the work is in the child. + """ + parent = tmp_path / "spawner.py" + parent.write_text( + textwrap.dedent(f""" + import subprocess, sys, time + p = subprocess.Popen([sys.executable, r"{burner_script}"]) + while p.poll() is None: + time.sleep(0.05) + """), + encoding="utf-8", + ) + + out_dir = tmp_path / "metrics" + rc = core.run_subprocess_with_monitoring( + command=[sys.executable, str(parent)], + working_dir=tmp_path, + stream_output=False, + metrics_interval=0.25, + output_dir=out_dir, + scenario_suffix="child", + log_output=False, + ) + assert rc == 0 + + rows = list( + csv.DictReader((out_dir / "metrics_child.csv").open(encoding="utf-8")) + ) + cpu = [ + float(r["cpu_percent_of_core"]) for r in rows if r["cpu_percent_of_core"] + ] + # Without the instance cache every descendant reads 0.0 on every sample, + # and the root here is deliberately idle, so the whole column is zero. + # Assert on the *proportion* of samples that registered work rather than + # on a level: that separates 0.0-always from working-but-throttled, + # which a magnitude threshold cannot do on a shared CI runner. + assert max(cpu) > 1.0, ( + "CPU was only counted for the idle root -- descendant Process " + "instances are not being cached, so their baselines never persist" + ) + # Drop the first sample: it is taken before the child has been primed. + # A third, not a half: on a slow runner the child can take a couple of + # samples to spawn and be primed. The defect yields zero non-zero + # samples, so any non-trivial fraction separates the two cases. + working = [c for c in cpu[1:] if c > 0.0] + assert len(working) >= len(cpu[1:]) / 3, ( + f"only {len(working)} of {len(cpu[1:])} samples registered child CPU; " + f"descendant baselines are not persisting across samples" + ) + procs = [int(r["num_processes"]) for r in rows if r["num_processes"]] + assert max(procs) >= 2, "the child process was never tracked" + def test_monitoring_disabled_writes_nothing(self, tmp_path, burner_script): out_dir = tmp_path / "metrics" rc = core.run_subprocess_with_monitoring( diff --git a/tests/test_cli.py b/tests/test_cli.py index 829d660..dbce097 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,6 +6,10 @@ import pytest +# config/config.yaml is untracked (machine-specific paths), so tests must never +# depend on it. The example config ships with the repo and always exists. +EXAMPLE_CONFIG = str(Path(__file__).parent.parent / "config" / "config.yaml.example") + def test_cli_help_command(): """Test that eddypro-batch --help returns zero and prints usage.""" @@ -97,6 +101,8 @@ def test_cli_scenarios_stub(): sys.executable, "-m", "eddypro_batch_processor.cli", + "--config", + EXAMPLE_CONFIG, "scenarios", "--rot-meth", "1", diff --git a/tests/test_cli_functions.py b/tests/test_cli_functions.py index cff3b7b..4a4467e 100644 --- a/tests/test_cli_functions.py +++ b/tests/test_cli_functions.py @@ -18,6 +18,11 @@ setup_logging, ) +# config/config.yaml is untracked (it holds machine-specific paths), so tests +# must never rely on it existing. The example config is version-controlled and +# is the only config guaranteed to be present in a fresh clone. +EXAMPLE_CONFIG = str(Path(__file__).parent.parent / "config" / "config.yaml.example") + def _write_ecmd_file(tmp_path: Path, site_id: str) -> Path: ecmd_path = tmp_path / "ecmd.csv" @@ -490,7 +495,7 @@ class TestMainFunction: def test_main_run_command(self): """Test main function routing to run command.""" - test_args = ["eddypro-batch", "run", "--dry-run"] + test_args = ["eddypro-batch", "--config", EXAMPLE_CONFIG, "run", "--dry-run"] with patch("sys.argv", test_args): with patch("eddypro_batch_processor.cli.cmd_run") as mock_cmd_run: @@ -503,7 +508,14 @@ def test_main_run_command(self): def test_main_scenarios_command(self): """Test main function routing to scenarios command.""" - test_args = ["eddypro-batch", "scenarios", "--rot-meth", "1"] + test_args = [ + "eddypro-batch", + "--config", + EXAMPLE_CONFIG, + "scenarios", + "--rot-meth", + "1", + ] with patch("sys.argv", test_args): with patch( @@ -518,7 +530,7 @@ def test_main_scenarios_command(self): def test_main_validate_command(self): """Test main function routing to validate command.""" - test_args = ["eddypro-batch", "validate"] + test_args = ["eddypro-batch", "--config", EXAMPLE_CONFIG, "validate"] with patch("sys.argv", test_args): with patch("eddypro_batch_processor.cli.cmd_validate") as mock_cmd_validate: @@ -531,7 +543,7 @@ def test_main_validate_command(self): def test_main_status_command(self): """Test main function routing to status command.""" - test_args = ["eddypro-batch", "status"] + test_args = ["eddypro-batch", "--config", EXAMPLE_CONFIG, "status"] with patch("sys.argv", test_args): with patch("eddypro_batch_processor.cli.cmd_status") as mock_cmd_status: