test: Refresh stale test fixtures OD-497 - #209
Conversation
Goldens snapshot live Codacy API and Trivy DB data that drifted since March, so CI was red on main before any change. Lizard's unit test now reads a checked-in pattern config instead of fetching defaults, making it independent of the API. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same upstream pattern drift as the other fixtures; regenerating this one needs a Codacy API token, so it was rebuilt from the CI diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
There was a problem hiding this comment.
Pull Request Overview
This PR successfully refreshes stale fixtures for Trivy, Lizard, PMD, Semgrep, Pylint, and Revive to stabilize CI. It introduces pinned configurations for Lizard to reduce external dependencies and updates PMD rulesets to the PMD 7 schema.
However, several inconsistencies were identified in the Lizard configuration files where explanation text and naming conventions do not match the updated thresholds or the PR description. Furthermore, a large-scale deletion of Semgrep security rules in the integration test expectations should be reviewed to ensure it doesn't represent a loss in coverage. Codacy reports that the changes are up to standards.
About this PR
- There is a large-scale removal of Semgrep security rules in 'integration-tests/config-discover/expected/tools-configs/semgrep.yaml' (lines 34259-34496). Please verify that this regeneration hasn't inadvertently removed valid rule discovery scenarios.
- Systemic discrepancy: Across the various Lizard configuration fixtures, the 'explanation' fields for rules like 'Lizard_parameter-count-medium' still reference the old default threshold (e.g., 5) despite the configuration being updated to 8. This should be synchronized across all affected files.
- The Trivy fixture updates are noted as a temporary fix. Consider a more robust strategy for handling daily vulnerability database updates in future iterations to prevent recurring CI staleness.
Test suggestions
- Verify Lizard unit tests use the newly pinned lizard.yaml instead of live API data
- Verify Trivy configuration discovery correctly includes go.sum
- Verify regenerated SARIF for Trivy correctly identifies current vulnerabilities in package-lock.json
- Verify PMD ruleset generation follows the PMD 7 schema as defined in expected XMLs
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| Lizard_parameter-count-medium: | ||
| category: Complexity | ||
| description: This rule checks the number of parameters passed to a function and raises an issue if it exceeds a medium threshold, which by default is 8 parameters. | ||
| description: Reports a Medium issue when a function's parameter count reaches the configured threshold, which defaults to 8. A signature this wide is hard to call correctly and usually signals that the function has taken on too many responsibilities. |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: The explanation for this rule (line 44) still mentions a default threshold of 5, which contradicts the updated description and configured value of 8. Update the explanation text to match the new value.
| # Test input, not a snapshot of Codacy's current Lizard patterns: it pins the pattern | ||
| # set so the SARIF assertion doesn't depend on the live API returning the same patterns. | ||
| patterns: | ||
| Lizard_ccn-medium: |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: This file uses 'Lizard_ccn-medium' despite the PR description stating that patterns were renamed to 'ccn-minor'. Please confirm if this is intentional for the unit test context.
Replaces #207, which was closed out as merged while unstacking the two PRs. Same content, now correctly based on
main.Changes
tools/lizard/test/lizard.yaml(new): pins the Lizard pattern set for the SARIF unit test. Without it the test fetched default patterns from the live API, so any upstream pattern change broke it.expected.sarifis unchanged.integration-tests/{init-without-token,init-with-token,config-discover}/expected/tools-configs/*: regenerated against the current API — trivy's file list gainedgo.sum, Lizard's patterns were renamed and re-levelled (ccn-medium→ccn-minor), and the opengrep/pmd/pylint/revive rule sets moved.plugins/tools/trivy/test/expected.sarif: regenerated — new CVEs in the Trivy DB added rules and shifted everyruleIndex.None of these are behaviour changes; the fixtures were last touched in March and the data behind them has moved since.
mainfails the same way today.Known remaining rot
plugins/tools/trivy/test/expected.sarifis compared against a vulnerability DB that updates daily, so this refresh buys days, not months. The opengrep tool test also failed one CI run on aconnection reset by peerwhile paginating patterns fromapp.codacy.com— a flake, not a fixture problem, but it shows how much of this suite depends on live network data. Both want a real fix (pinned DB / assert on a subset), tracked separately.Manual Testing