You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "hanahmily (via GitHub)" <gi...@apache.org> on 2026/05/21 00:59:28 UTC
[PR] fix(test/cases/measure): preserve Apache license header on captured want fixtures and gate the capture loop [skywalking-banyandb]
hanahmily opened a new pull request, #1137:
URL: https://github.com/apache/skywalking-banyandb/pull/1137
## Summary
- The capture writers (`test/cases/measure/cmd/capture/capture_test.go:172` and `cmd/generate/capture.go:168`) wrote `want/*.yaml` directly from `yaml.JSONToYAML(respJSON)` — every regeneration silently stripped the Apache 2.0 license header that `make license-check` requires. Move the `licenseHeader` const out of the `main` package into the importable `test/cases/measure/data` package as `data.LicenseHeader`, and prepend it on every write.
- Gate `TestCapture` behind `CAPTURE_WANT_FIXTURES=1`. Routine `./...` discovery (e.g. `make test` run from the `test/` project dir) was silently invoking it and re-baselining the golden fixtures against the current branch's output. With the gate, the test `t.Skip`s in ~10 ms unless explicitly opted in. The new `make capture-test-cases` target sets the env var and runs it; that is now the documented entry point.
- Also ignore `.omx/` alongside `.omc/` in `.gitignore` — local scratch dir for OMC/Anthropic tooling artifacts (bench reports, profiler dumps, session logs).
### Why the bug was dormant on `main`
CI's test matrix targets `./banyand/...`, `./pkg/...`, and the two integration suites. None of them traverse `./test/cases/...`, so `TestCapture` only runs when a developer explicitly invokes it — or, more dangerously, when `make test` is run from the `test/` project dir and Ginkgo discovers the `gen_*` test cases. The latter is what surfaced this on a feature branch.
## Test plan
- [x] `go test -count=1 -v -run TestCapture ./test/cases/measure/cmd/capture/` — `--- SKIP: TestCapture (0.00s)`, no fixture rewrite.
- [x] `make capture-test-cases` — runs in ~2.7 s, every produced `want/*.yaml` starts with `# Licensed to Apache Software Foundation`.
- [x] `make license-check` — clean for `test/cases/measure/data/want/*.yaml` after regeneration.
- [x] `go build ./...` — clean.
- [x] `golangci-lint run ./cases/measure/cmd/... ./cases/measure/data/...` (from `test/`) — clean.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(test/cases/measure): preserve Apache license header on captured want fixtures and gate the capture loop [skywalking-banyandb]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1137:
URL: https://github.com/apache/skywalking-banyandb/pull/1137#issuecomment-4503916663
## [Codecov](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1137?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 39.88%. Comparing base ([`3530dd9`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/3530dd9b5cefaa4fef60eec91ecbfbdfcaf243c5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`0e2280a`](https://app.codecov.io/gh/apache/skywalking-banyandb/commit/0e2280ae326450e08ef383ca423b98f1261256d5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 258 commits behind head on main.
> :exclamation: There is a different number of reports uploaded between BASE (3530dd9) and HEAD (0e2280a). Click for more details.
>
> <details><summary>HEAD has 1 upload less than BASE</summary>
>
>| Flag | BASE (3530dd9) | HEAD (0e2280a) |
>|------|------|------|
>||1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #1137 +/- ##
==========================================
- Coverage 45.97% 39.88% -6.10%
==========================================
Files 328 181 -147
Lines 55505 17487 -38018
==========================================
- Hits 25520 6974 -18546
+ Misses 27909 9864 -18045
+ Partials 2076 649 -1427
```
| [Flag](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1137/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [pkg](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1137/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.88% <ø> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/skywalking-banyandb/pull/1137?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(test/cases/measure): preserve Apache license header on captured want fixtures and gate the capture loop [skywalking-banyandb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #1137:
URL: https://github.com/apache/skywalking-banyandb/pull/1137#discussion_r3278054810
##########
test/cases/measure/cmd/generate/capture.go:
##########
@@ -165,7 +165,8 @@ func runCapture(outputDir, serverAddr string) {
}
wantPath := filepath.Join(wantDirPath, testName+".yaml")
- if writeErr := os.WriteFile(wantPath, respYAML, 0o600); writeErr != nil {
+ wantContent := append([]byte(licenseHeader), respYAML...)
+ if writeErr := os.WriteFile(wantPath, wantContent, 0o600); writeErr != nil {
fmt.Fprintf(os.Stderr, " [ERROR] %s: write failed: %v\n", testName, writeErr)
Review Comment:
`append([]byte(licenseHeader), respYAML...)` converts the same header string to a new byte slice on every iteration, adding avoidable allocations during capture. Consider precomputing the header as `[]byte` once (outside the loop) and then constructing `wantContent` with a single allocation (e.g., via `make`+`copy`) to keep capture runs more efficient.
##########
test/cases/measure/cmd/capture/capture_test.go:
##########
@@ -169,7 +185,8 @@ func TestCapture(t *testing.T) {
}
wantPath := filepath.Join(wantDirPath, testName+".yaml")
- if writeErr := os.WriteFile(wantPath, respYAML, 0o600); writeErr != nil {
+ wantContent := append([]byte(casesMeasureData.LicenseHeader), respYAML...)
+ if writeErr := os.WriteFile(wantPath, wantContent, 0o600); writeErr != nil {
t.Errorf(" [ERROR] %s: write failed: %v", testName, writeErr)
Review Comment:
`append([]byte(casesMeasureData.LicenseHeader), respYAML...)` creates a new `[]byte` copy of the header on every want-file write. Since this loop can write many fixtures, consider caching the header bytes once before the loop and composing each `wantContent` with a single allocation (e.g., `make`+`copy`) to reduce allocations and GC pressure.
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(test/cases/measure): preserve Apache license header on captured want fixtures and gate the capture loop [skywalking-banyandb]
Posted by "hanahmily (via GitHub)" <gi...@apache.org>.
hanahmily merged PR #1137:
URL: https://github.com/apache/skywalking-banyandb/pull/1137
--
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: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org