From 6ce6a1deceadf1b1b9d4e98af8d8072b01ef8eb0 Mon Sep 17 00:00:00 2001 From: Kienne <152148378+hopkienne@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:53:55 +0700 Subject: [PATCH] fix(salary): reject non-finite converter baselines (#665) * fix(salary): reject non-finite converter baselines * changelog: place the #665 entry under [Unreleased] Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: GAPIT-KIENTH\kienth Co-authored-by: Mads Lorentzen Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 7 ++++ tests/test_salary_baseline_cli.py | 57 +++++++++++++++++++++++++++++++ tools/convert_salary_excel.py | 3 ++ 3 files changed, 67 insertions(+) create mode 100644 tests/test_salary_baseline_cli.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ec8db9f7..64f839f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -108,6 +108,13 @@ per-file diff commands. in `salary_data.json` and `--list-all`. A fallback-detected header with no salary column beside it now skips the sheet with a warning; strictly detected headers are unchanged. +- **`convert_salary_excel.py` rejects a non-finite `--baseline`** (#665, + `tools/convert_salary_excel.py`, `tests/test_salary_baseline_cli.py`) - `nan`, `inf`, `-inf` and + an overflow such as `1e309` were accepted with exit 0 and written as `index_baseline`, + overwriting a good `salary_data.json` with one that `salary_lookup.py --validate` rejects; every + lookup then failed and `/apply` quietly dropped the benchmark. The converter now exits 2 before + reading the workbook or opening the output, and `0` or a finite custom baseline still converts. + ## [1.7.3] - 2026-10-19 ### Added diff --git a/tests/test_salary_baseline_cli.py b/tests/test_salary_baseline_cli.py new file mode 100644 index 00000000..ff915b73 --- /dev/null +++ b/tests/test_salary_baseline_cli.py @@ -0,0 +1,57 @@ +"""The converter must not write baselines rejected by salary_lookup --validate.""" +import json +import subprocess +import sys +import unittest +from pathlib import Path +from tempfile import TemporaryDirectory + +try: + import openpyxl +except ImportError: + openpyxl = None + +TOOL = Path(__file__).resolve().parents[1] / "tools" / "convert_salary_excel.py" + + +@unittest.skipUnless(openpyxl, "requires optional openpyxl (installed in CI)") +class BaselineCLI(unittest.TestCase): + def setUp(self): + directory = TemporaryDirectory() + self.addCleanup(directory.cleanup) + self.root = Path(directory.name) + self.workbook = self.root / "salary.xlsx" + self.output = self.root / "salary.json" + book = openpyxl.Workbook() + try: + book.active.append(["Company", "Count", "Index"]) + book.active.append(["Acme", 12, 108.5]) + book.save(self.workbook) + finally: + book.close() + + def convert(self, baseline): + return subprocess.run( + [sys.executable, str(TOOL), str(self.workbook), + "--output", str(self.output), f"--baseline={baseline}"], + capture_output=True, text=True, encoding="utf-8", + ) + + def test_nonfinite_baseline_is_rejected_without_overwriting_output(self): + for baseline in ("nan", "inf", "-inf", "1e309"): + with self.subTest(baseline=baseline): + self.output.write_text("existing salary data\n", encoding="utf-8") + result = self.convert(baseline) + self.assertNotEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("--baseline", result.stderr) + self.assertIn("finite", result.stderr) + self.assertNotIn("Traceback", result.stderr) + self.assertEqual(self.output.read_text(encoding="utf-8"), "existing salary data\n") + + def test_finite_custom_and_zero_baselines_are_preserved(self): + for baseline in ("150", "0"): + with self.subTest(baseline=baseline): + result = self.convert(baseline) + self.assertEqual(result.returncode, 0, result.stderr) + data = json.loads(self.output.read_text(encoding="utf-8")) + self.assertEqual(data["metadata"]["index_baseline"], float(baseline)) diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index 11362267..f844552e 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -398,6 +398,9 @@ def main(): ) args = parser.parse_args() + if not math.isfinite(args.baseline): + parser.error("--baseline must be a finite number") + excel_path = Path(args.excel_file) if not excel_path.exists(): print(f"Error: File not found: {excel_path}", file=sys.stderr)