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/pkgsite/internal/middleware/stats: TestStats failures #61770

Closed
gopherbot opened this issue Aug 4, 2023 · 10 comments
Closed

x/pkgsite/internal/middleware/stats: TestStats failures #61770

gopherbot opened this issue Aug 4, 2023 · 10 comments
Assignees
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. pkgsite Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@gopherbot
Copy link
Contributor

gopherbot commented Aug 4, 2023

#!watchflakes
post <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats" && `MillisTo.* is \d+, wanted \d+`

Issue created automatically to collect these failures.

Example (log):

--- FAIL: TestStats (0.72s)
    stats_test.go:72: MillisToLastByte is 696, wanted 500 +/- 50

watchflakes

@gopherbot gopherbot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 4, 2023
@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats"
2023-08-04 20:52 dragonfly-amd64-622 pkgsite@b486dfa0 go@a4b6685c x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.72s)
    stats_test.go:72: MillisToLastByte is 696, wanted 500 +/- 50
2023-08-04 20:52 netbsd-amd64-9_3 pkgsite@b486dfa0 go@a4b6685c x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.55s)
    stats_test.go:72: MillisToLastByte is 551, wanted 500 +/- 50

watchflakes

@gopherbot gopherbot added this to the Unreleased milestone Aug 4, 2023
@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats"
2023-08-04 20:52 linux-s390x-ibm pkgsite@b486dfa0 go@a4b6685c x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.84s)
    stats_test.go:72: MillisToLastByte is 606, wanted 500 +/- 50

watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats"
2023-08-04 21:05 netbsd-amd64-9_3 pkgsite@52eb2284 go@ea565f15 x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.57s)
    stats_test.go:72: MillisToLastByte is 563, wanted 500 +/- 50

watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats"
2023-08-04 21:05 netbsd-amd64-9_3 pkgsite@52eb2284 go@02366863 x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.58s)
    stats_test.go:72: MillisToLastByte is 579, wanted 500 +/- 50

watchflakes

@bcmills bcmills added pkgsite Testing An issue that has been verified to require only test changes, not just a test failure. labels Aug 7, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 7, 2023

This is just a fragile test. It uses a hard-coded time.Sleep(500 * time.Millisecond):
https://cs.opensource.google/go/x/pkgsite/+/master:internal/middleware/stats/stats_test.go;l=29;drc=9e6bdc9d522da9b09475342d9d7629daae359042
and assumes that it will run within a hard-coded 50ms tolerance:
https://cs.opensource.google/go/x/pkgsite/+/master:internal/middleware/stats/stats_test.go;l=67;drc=9e6bdc9d522da9b09475342d9d7629daae359042

The test should instead measure the actual timings and set the tolerance accordingly. For example, it can call time.Now() before and after relevant Write calls (or before and/or after ts.Client().Get) to determine precise bounds on the timings that do not rely on any assumptions about scheduling delays on the machine running the test.

(CC @matloob @hyangah)

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 7, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 7, 2023
@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
default <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats"
2023-08-07 18:54 linux-s390x-ibm pkgsite@3918c803 go@008ab9ad x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.57s)
    stats_test.go:72: MillisToLastByte is 568, wanted 500 +/- 50
2023-08-08 00:53 linux-s390x-ibm pkgsite@c3163ea5 go@adb775e3 x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.70s)
    stats_test.go:72: MillisToLastByte is 693, wanted 500 +/- 50

watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats" && `MillisTo.* is \d+, wanted \d+`
2023-08-08 00:53 netbsd-386-9_3 pkgsite@c3163ea5 go@3393155a x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.57s)
    stats_test.go:72: MillisToLastByte is 567, wanted 500 +/- 50

watchflakes

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats" && `MillisTo.* is \d+, wanted \d+`
2023-08-08 00:53 netbsd-386-9_3 pkgsite@c3163ea5 go@6e434079 x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.58s)
    stats_test.go:72: MillisToLastByte is 581, wanted 500 +/- 50
2023-08-08 22:30 linux-s390x-ibm pkgsite@a3b12eae go@c19c4c56 x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.91s)
    stats_test.go:72: MillisToLastByte is 908, wanted 500 +/- 50

watchflakes

@findleyr
Copy link
Member

This was fixed by https://go.dev/cl/517975.

@gopherbot
Copy link
Contributor Author

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "golang.org/x/pkgsite/internal/middleware/stats" && test == "TestStats" && `MillisTo.* is \d+, wanted \d+`
2023-08-08 00:53 linux-s390x-ibm pkgsite@c3163ea5 go@6e434079 x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (1.01s)
    stats_test.go:72: MillisToLastByte is 1008, wanted 500 +/- 50
2023-08-08 22:30 linux-s390x-ibm pkgsite@a3b12eae go@288fd6eb x/pkgsite/internal/middleware/stats.TestStats (log)
--- FAIL: TestStats (0.73s)
    stats_test.go:72: MillisToLastByte is 732, wanted 500 +/- 50

watchflakes

@golang golang locked and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. pkgsite Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
Archived in project
Development

No branches or pull requests

4 participants