Repository navigation
Conversation
|
Thanks for opening your first pull request in the Codna CLI! Please make sure the checks are green and that the change follows CONTRIBUTING.md. A maintainer will review it shortly. |
![codna-ai[bot]](https://avatars.githubusercontent.com/in/4061960?s=60&v=4){kind=small}
| [sys.executable, "-m", "pytest", "-q", f"--junitxml={suite_junit}"], | ||
| suite_junit, | ||
| per_suite, | ||
| )) |
There was a problem hiding this comment.
🔴 HIGH · correctness — Removed --ignore flags cause nested suites to be double-collected
The diff deletes the --ignore arguments that prevented a parent pytest suite from recursively collecting tests in child suite directories. Previously: argv.extend(f"--ignore={candidate}" for candidate in suites if os.path.dirname(candidate) == suite). Without these flags, when suite A is an ancestor of suite B in the file system (e.g., repo/tests and repo/tests/integration), running pytest on suite A will also collect all tests under suite B. Those tests are then run and reported again when suite B is processed. This causes duplicate test IDs in the failing list, inflated failure counts, and incorrect len(failing) in the returned summary.
| [sys.executable, "-m", "pytest", "-q", f"--junitxml={suite_junit}"], | |
| suite_junit, | |
| per_suite, | |
| )) | |
| Restore the --ignore= arguments: | |
| ```python | |
| argv = [sys.executable, "-m", "pytest", "-q"] | |
| argv.extend(f"--ignore={candidate}" for candidate in suites if os.path.dirname(candidate) == suite) | |
| argv.append(f"--junitxml={suite_junit}") | |
| runs.append((suite, argv, suite_junit, per_suite)) |
_Reply `@codna fix` and Codna opens a verified, test-green, risk-gated fix PR._
<!-- codna:review:fp=b1f30f6472f67457 -->
<!-- codna:finding eyJmcCI6ICJiMWYzMGY2NDcyZjY3NDU3IiwgInBhdGgiOiAiY29kbmEvdGVzdHJ1bi5weSIsICJsaW5lIjogMzU3LCAiZW5kX2xpbmUiOiAzNjAsICJzZXZlcml0eSI6ICJoaWdoIiwgImNhdGVnb3J5IjogImNvcnJlY3RuZXNzIiwgInRpdGxlIjogIlJlbW92ZWQgLS1pZ25vcmUgZmxhZ3MgY2F1c2UgbmVzdGVkIHN1aXRlcyB0byBiZSBkb3VibGUtY29sbGVjdGVkIiwgImV4cGxhbmF0aW9uIjogIlRoZSBkaWZmIGRlbGV0ZXMgdGhlIGAtLWlnbm9yZWAgYXJndW1lbnRzIHRoYXQgcHJldmVudGVkIGEgcGFyZW50IHB5dGVzdCBzdWl0ZSBmcm9tIHJlY3Vyc2l2ZWx5IGNvbGxlY3RpbmcgdGVzdHMgaW4gY2hpbGQgc3VpdGUgZGlyZWN0b3JpZXMuIFByZXZpb3VzbHk6IGBhcmd2LmV4dGVuZChmXCItLWlnbm9yZT17Y2FuZGlkYXRlfVwiIGZvciBjYW5kaWRhdGUgaW4gc3VpdGVzIGlmIG9zLnBhdGguZGlybmFtZShjYW5kaWRhdGUpID09IHN1aXRlKWAuIFdpdGhvdXQgdGhlc2UgZmxhZ3MsIHdoZW4gc3VpdGUgQSBpcyBhbiBhbmNlc3RvciBvZiBzdWl0ZSBCIGluIHRoZSBmaWxlIHN5c3RlbSAoZS5nLiwgYHJlcG8vdGVzdHNgIGFuZCBgcmVwby90ZXN0cy9pbnRlZ3JhdGlvbmApLCBydW5uaW5nIHB5dGVzdCBvbiBzdWl0ZSBBIHdpbGwgYWxzbyBjb2xsZWN0IGFsbCB0ZXN0cyB1bmRlciBzdWl0ZSBCLiBUaG9zZSB0ZXN0cyBhcmUgdGhlbiBydW4gYW5kIHJlcG9ydGVkIGFnYWluIHdoZW4gc3VpdGUgQiBpcyBwcm9jZXNzZWQuIFRoaXMgY2F1c2VzIGR1cGxpY2F0ZSB0ZXN0IElEcyBpbiB0aGUgYGZhaWxpbmdgIGxpc3QsIGluZmxhdGVkIGZhaWx1cmUgY291bnRzLCBhbmQgaW5jb3JyZWN0IGBsZW4oZmFpbGluZylgIGluIHRoZSByZXR1cm5lZCBzdW1tYXJ5LiJ9 -->
| ) | ||
| parts.append(f"{os.path.relpath(suite, repo_dir)}: {message}") | ||
| for suite in crashed: | ||
| parts.append(f"{os.path.relpath(suite, repo_dir)}: exited non-zero with no JUnit report") |
There was a problem hiding this comment.
🟡 MEDIUM · correctness — Crashed-suite diagnostic always reports 'no JUnit report' even when one exists
The old code stored (run_dir, os.path.isfile(run_junit)) in crashed and produced a different message when a JUnit file was present ("exited non-zero with JUnit report but no failing test ids") versus absent ("exited non-zero with no JUnit report"). The diff simplifies crashed to list[str] and unconditionally emits "exited non-zero with no JUnit report". A suite that exits non-zero but produces a JUnit report without failing test cases (e.g., plugin errors, teardown errors, or the file was written before the crash) is now misdiagnosed. Callers parsing the returned message will take the wrong remediation path.
Reply @codna fix and Codna opens a verified, test-green, risk-gated fix PR.
| # Each suite runs from its own root so its pyproject/pytest.ini applies; the overall budget | ||
| # is split evenly (deterministic). The 60s floor is safe: _MAX_SUITES caps the count, so | ||
| # floor * suites can never exceed 8*60s = 480s even when timeout_seconds is small. | ||
| per_suite = max(60, timeout_seconds // len(suites)) |
There was a problem hiding this comment.
🟡 MEDIUM · correctness — Per-suite budget floor can cause total wall time to exceed timeout_seconds
per_suite = max(60, timeout_seconds // len(suites)) — when timeout_seconds < 60 * len(suites), each suite is allocated 60 s and the aggregate budget becomes 60 * len(suites), which can be up to 60 * 8 = 480 s regardless of what timeout_seconds was. A caller that passes timeout_seconds=120 with 4 suites will wait up to 240 s before the last suite times out, violating the contract. The comment's reasoning ('floor * suites can never exceed 8*60s') describes an absolute maximum but not a bound relative to timeout_seconds.
Reply @codna fix and Codna opens a verified, test-green, risk-gated fix PR.
| "LocalRuntime collection — open it with the matching runtime (TELYS_RUNTIME), or rebuild it." | ||
| ) | ||
| V = np.load(base_path, allow_pickle=False) | ||
| meta = json.load(open(os.path.join(path, "ty_native.json"))) |
There was a problem hiding this comment.
⚪ LOW · correctness — open_index opens ty_native.json without closing the file handle
meta = json.load(open(os.path.join(path, "ty_native.json"))) — the file handle from open() is never assigned to a variable and therefore never explicitly closed. On CPython this leaks the handle until GC runs; on other runtimes it leaks indefinitely. If open_index is called many times (e.g., during testing or re-loading), this can exhaust file descriptors.
| meta = json.load(open(os.path.join(path, "ty_native.json"))) | |
| Use a `with` statement: `with open(os.path.join(path, "ty_native.json")) as f: meta = json.load(f)` |
Reply @codna fix and Codna opens a verified, test-green, risk-gated fix PR.
Automated mirror of cli/ (flattened), the root allowlist (server.json, glama.json, LICENSE, .gitleaks.toml) and the public-root/ community tree from thyn-ai/codna at 3177a0b861e68aede678cf2c28e8697690cf3855. Merges itself via auto-merge once the mirror's required checks (codna review, Analyze (python)) pass and the codna-ai review approves; if a later sync lands first, this PR is closed as superseded -- every sync PR is a full snapshot of the exported surface.