Conversation
Use the generator to generate the golden files...duh
Fix additional pre-commit issues
clean up assertions
Greptile SummaryAdds a golden-file regression test suite for
Confidence Score: 4/5Safe to merge with the noted test gap addressed; the infrastructure is solid but the missing inverse check leaves a real CI blind-spot. One P1 finding (no test catches a dataset file whose stub has no golden), two P2 findings (cleanup gap and incomplete tests/units/reflex_base/utils/pyi_generator/test_regression.py — Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[pytest collects test_regression.py] --> B[_existing_golden_cases\nscans golden/*.pyi at collection time]
B --> C{Golden files exist?}
C -- No --> D[No test_pyi_golden cases collected]
C -- Yes --> E[Parametrize test_pyi_golden\nper golden file]
E --> F[generated_stubs fixture\nscope=module]
F --> G[PyiGenerator.scan_all\nwrites .pyi into DATASET_DIR]
G --> H[Read each .pyi to results dict]
H --> I[pyi_path.unlink for each file]
I --> J[Return source to content map]
J --> K[test_pyi_golden\ncompare normalized stub vs golden]
J --> L[test_no_extra_golden_files\nexisting goldens subset of expected_goldens]
subgraph gap [Missing test]
M[test_all_generated_stubs_have_goldens\ngenerated_stubs keys subset of golden files on disk]
end
J -.->|not implemented| M
N[CLI: python -m ... --update] --> O[update_golden_files\ncalls _run_generator]
O --> P[Write normalized stubs to golden/]
P --> Q[Remove stale golden files]
Reviews (1): Last reviewed commit: "fix ruff issues in dataset" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| """Ensure no golden files exist without corresponding dataset sources. | ||
|
|
||
| Args: | ||
| generated_stubs: The mapping of dataset source paths to generated .pyi content. | ||
| """ | ||
| expected_goldens = {_golden_path_for(s) for s in generated_stubs} | ||
| for existing in GOLDEN_DIR.rglob("*.pyi"): | ||
| assert existing in expected_goldens, ( | ||
| f"Stale golden file {existing.relative_to(_HERE)} has no dataset source. " | ||
| f"Run `{_UPDATE_CMD}` to clean up." |
There was a problem hiding this comment.
test_no_extra_golden_files checks the one direction (every existing golden has a stub), but the inverse is never checked: when a new dataset file generates a stub and no golden yet exists, it simply doesn't appear in GOLDEN_DIR.rglob("*.pyi") and no assertion ever fires. A developer could add a dataset file, skip running --update, and CI would stay green while the new scenario goes untested.
def test_all_generated_stubs_have_golden_files(generated_stubs: dict[Path, str]):
"""Ensure every generated stub has a corresponding golden file."""
for source_path in sorted(generated_stubs):
golden_path = _golden_path_for(source_path)
assert golden_path.exists(), (
f"No golden file for {source_path.name}. "
f"Run `{_UPDATE_CMD}` to generate."
)| Returns: | ||
| A mapping from dataset source .py files to their generated .pyi content. | ||
| """ | ||
| gen = PyiGenerator() | ||
| gen.scan_all([str(DATASET_DIR)]) | ||
|
|
||
| results: dict[Path, str] = {} | ||
| for pyi_str, _hash in gen.written_files: | ||
| pyi_path = Path(pyi_str) | ||
| source_path = pyi_path.with_suffix(".py") | ||
| results[source_path] = pyi_path.read_text() | ||
| pyi_path.unlink(missing_ok=True) | ||
| return results | ||
|
|
||
|
|
There was a problem hiding this comment.
.pyi files on failure
_run_generator calls pyi_path.unlink(missing_ok=True) inside the loop, so if an exception fires after scan_all but before all unlink calls complete (e.g., a read error on one file), the remaining generated .pyi files stay in DATASET_DIR indefinitely. There's also no .gitignore in the dataset directory to prevent accidentally staging them. A try/finally would guarantee cleanup:
pyi_files: list[Path] = []
try:
for pyi_str, _hash in gen.written_files:
pyi_path = Path(pyi_str)
pyi_files.append(pyi_path)
source_path = pyi_path.with_suffix(".py")
results[source_path] = pyi_path.read_text()
finally:
for pyi_path in pyi_files:
pyi_path.unlink(missing_ok=True)| ) | ||
| parser.add_argument( | ||
| "--check", | ||
| action="store_true", | ||
| help="Check that generated stubs match golden files (CI mode).", | ||
| ) | ||
| args = parser.parse_args() | ||
|
|
||
| if args.update: | ||
| print(f"Regenerating golden files from {DATASET_DIR} ...") | ||
| updated = update_golden_files() | ||
| print( | ||
| f"\nDone. {len(updated)} file(s) updated in {GOLDEN_DIR.relative_to(_HERE)}/" | ||
| ) | ||
| print("Review the changes and commit them with your PR.") | ||
| elif args.check: | ||
| print("Checking generated stubs against golden files...") | ||
| generated = _run_generator() | ||
| failures = [] | ||
| for source_path, content in sorted(generated.items()): | ||
| golden_path = _golden_path_for(source_path) | ||
| if not golden_path.exists(): | ||
| continue | ||
| if _normalize_stub(content) != golden_path.read_text(): | ||
| failures.append(f" {source_path.name}: differs from golden") | ||
| if failures: | ||
| print("FAILED:") |
There was a problem hiding this comment.
--check mode is silent about stale golden files
The --check CLI path skips missing goldens (if not golden_path.exists(): continue) but never reports the inverse: golden files that have no corresponding generated stub. test_no_extra_golden_files catches stale goldens in pytest, but the --check mode used in CI-style runs doesn't replicate this check, so the two modes can diverge.
2 participants
There are some issues present in the dataset that represent real issues in the pyi_generator that need to be fixed. Wanted to get the regression test in before fixing those, so we can see how the files change in relation to the fixes.