test: fix BenchmarkNova clone setup#57
Conversation
Clone nova once over HTTPS, use b.Cleanup for teardown, and measure only WalkDir in the benchmark loop. Closes dolph#16 Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c8aa36eeb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| b.StopTimer() | ||
| d := cloneRepoToBenchDir(b, repoURL) | ||
| fr := findReplace{find: RandomString(2), replace: RandomString(2)} | ||
| b.ResetTimer() |
There was a problem hiding this comment.
BenchmarkNova stops the timer for setup and then calls ResetTimer, but ResetTimer does not start timing again. As written, the loop executes with timing disabled, so WalkDir work is not measured and the benchmark can report misleading numbers (and may over-calibrate b.N because elapsed benchmark time stays near zero). Add b.StartTimer() before entering the loop (or avoid StopTimer/ResetTimer here).
Useful? React with 👍 / 👎.
| d := cloneRepoToBenchDir(b, repoURL) | ||
| fr := findReplace{find: RandomString(2), replace: RandomString(2)} | ||
| b.ResetTimer() | ||
|
|
||
| for n := 0; n < b.N; n++ { |
There was a problem hiding this comment.
This benchmark now clones Nova once and then reuses the same directory for all iterations, but WalkDir mutates file contents and names via replace/rename operations. That means iteration 2+ run on already-modified input, so the measured ns/op no longer reflects the cost on a fresh checkout and will typically trend artificially low after the first pass. Recreate/reset the repo per iteration outside timed sections to keep workloads consistent.
Useful? React with 👍 / 👎.
1 participant
Summary
openstack/novaonce per benchmark run over HTTPS (no SSH keys required).b.Cleanupfor temp dir teardown instead of a brokendeferinside the helper.WalkDirin the timed loop.Closes #16
Made with Cursor