rusackas commented on code in PR #332: URL: https://github.com/apache/superset-kubernetes-operator/pull/332#discussion_r3936476323
########## docs/contributing/development-guidelines.md: ########## @@ -188,13 +188,13 @@ make fuzz # 30s per target make fuzz FUZZTIME=2m # longer local run ``` -Each target seeds a corpus with `f.Add(...)` cases and asserts an invariant beyond "does not panic" — e.g. `RenderConfig` is deterministic, `pyQuote` round-trips, `MergeMaps` is a last-writer-wins union. The seed corpus (plus any committed `testdata/fuzz/...` reproducers) replays as ordinary subtests during `make test-unit`, so regressions are caught on every PR; the scheduled `fuzz.yaml` workflow runs the targets for longer to explore new inputs. +Each target seeds a corpus with `f.Add(...)` cases and asserts an invariant beyond "does not panic" — e.g. `RenderConfig` is deterministic, `pyQuote` round-trips, `MergeMaps` is a last-writer-wins union. The seed corpus replays as ordinary subtests during `make test-unit`, so regressions are caught on every PR; the scheduled `fuzz.yaml` workflow runs the targets for longer to explore new inputs. **When to add a target:** a pure, deterministic function that parses strings, generates code/config, or merges collections from CR-author-controlled input. Skip trivial scalar or preset math already covered by table tests. **Scope — robustness, not a security boundary.** As noted under [Security and the threat model](#security-and-the-threat-model), `Superset` CR input comes from a *trusted* namespace admin. Fuzzing here guards against panics and non-determinism on awkward-but-valid input; it is not defending a trust boundary. Frame findings accordingly. -**On a finding:** `go test` writes a reproducer to `testdata/fuzz/<target>/<id>`. Commit it as a permanent regression seed, then fix the bug. +**On a finding:** `go test` writes a reproducer to `testdata/fuzz/<target>/<id>`. Capture its minimized input and add it back as an `f.Add(...)` seed in the target (with a short comment noting it is a fuzz-discovered regression), then delete the `testdata/fuzz/<target>/<id>` file and fix the bug. Committing the seed as an `f.Add(...)` line — rather than the raw `testdata/fuzz` reproducer — keeps it replaying on every `make test-unit` while staying license-headed Go source: the generated corpus files carry no Apache license header and fail the `check-license` (Apache Rat) job, and Rat cannot reliably exclude them because it matches on their hash-based filenames. Review Comment: Tiny nit: I poked at this locally and a bare directory entry like `fuzz` (or `testdata`) in `.rat-excludes` does exclude the whole subtree, so Rat *can* skip them... it's the path-style regexes that don't match. The `f.Add` approach still seems like the right convention regardless, so I'd probably just trim that last clause rather than carve out a blanket exclusion. Up to you, not blocking. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
