Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/telemetry/internal/regtest: test consistently failing on many platforms since CL 560462 #65544

Closed
bcmills opened this issue Feb 6, 2024 · 6 comments
Assignees
Labels
Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) telemetry x/telemetry issues
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 6, 2024

#!watchflakes
post <- pkg == "golang.org/x/telemetry/internal/regtest" && (
	goarch == "386" ||
	goarch == "wasm" ||
	goos == "android" ||
	goos == "illumos" ||
	goos == "solaris" ||
	goos == "openbsd" ||
	goos == "plan9")

Go version

9819d82

Output of go env in your module/workspace:

N/A

What did you do?

Check builder status for #11811.

What did you see happen?

https://build.golang.org/log/3ea3d15a5b41086aeb875a6846533cb78175b83b:

--- FAIL: TestE2E (0.00s)
    e2e_test.go:145: unmet expectation:
        got {}
        want {
        	 "counter": 1,
        	 "gopls/editor:expected": 1,
        	 "stack/expected\n": 1
        	}
    e2e_test.go:152: unmet expectation:
        got {}
        want {
        	 "counter:surprise": 1,
        	 "gopls/editor:surprise": 1,
        	 "stack-surprise\n": 1
        	}
FAIL
FAIL	golang.org/x/telemetry/internal/regtest	0.024s

This affects all 386 platforms, including the first class ports for linux/386 and windows/386.

Similar failures occur on the android, illumos, js, openbsd, plan9, solaris, and wasip1 builders, although since those are secondary ports perhaps they are just misconfigured builders.

What did you expect to see?

All tests should be either passing or skipped, especially on first class ports such as linux/386 and windows/386.

For secondary ports that are intentionally not supported, either the tests should be skipped or the builders should be configured not to run them in the first place.

@gopherbot gopherbot added the telemetry x/telemetry issues label Feb 6, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 6, 2024
@bcmills bcmills added the Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) label Feb 6, 2024
@bcmills
Copy link
Contributor Author

bcmills commented Feb 6, 2024

It is also worth noting that this test does not meet the standard of https://go.dev/wiki/CodeReviewComments#useful-test-failures: it shows what was got and what was expected, but it does not describe “with what inputs” and doesn't report any errors encountered by the telemetry collection & upload process.

@findleyr
Copy link
Contributor

findleyr commented Feb 6, 2024

I believe @pjweinb has a fix for this in https://go.dev/cl/561995.

CC @hyangah @matloob

@findleyr
Copy link
Contributor

findleyr commented Feb 6, 2024

Nevermind, that CL removes the proxy API which I still believe is necessary. I'll send a fix for the builder breakage.

@findleyr findleyr self-assigned this Feb 6, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/562076 mentions this issue: internal/regtest: skip TestE2E if telemetry is disabled on platform

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/562396 mentions this issue: counter/countertest: add returns to disabled functions that need them

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/563359 mentions this issue: counter: add illumos to the unsupported platform list

gopherbot pushed a commit to golang/telemetry that referenced this issue Feb 13, 2024
For golang/go#65544
For golang/go#65549

Change-Id: Icb8d598e419e6b34c184642a851a699be8b76382
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/563359
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Pinkle-pash added a commit to Pinkle-pash/telemetry that referenced this issue Dec 4, 2024
Also add a stub version of the countertest package similar to the one
in counter_disabled.go so that countertest can compile on platforms
where telemetry disabled.

Fixes golang/go#65544

Change-Id: I46272034a4d80fe3c79fd5bfd9073eb5ef3b6010
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/562076
Reviewed-by: Robert Findley <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Pinkle-pash added a commit to Pinkle-pash/telemetry that referenced this issue Dec 4, 2024
Also add a SkipIfUnsupportedPlatform to TestE2E_off

Fixes golang/go#65544

Change-Id: I8896c8c4c80589516fefd6357cfdf302cd93945b
Cq-Include-Trybots: luci.golang.try:x_telemetry-gotip-linux-386,x_telemetry-gotip-js-wasm,x_telemetry-gotip-solaris-amd64,x_telemetry-gotip-openbsd-amd64,x_telemetry-gotip-windows-386,x_telemetry-gotip-wasip1-wasm_wazero
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/562396
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Pinkle-pash added a commit to Pinkle-pash/telemetry that referenced this issue Dec 4, 2024
For golang/go#65544
For golang/go#65549

Change-Id: Icb8d598e419e6b34c184642a851a699be8b76382
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/563359
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Hyang-Ah Hana Kim <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) telemetry x/telemetry issues
Projects
None yet
Development

No branches or pull requests

4 participants