Relax benchmark test assertions to fix test failures on slower hardware#21
Open
ottok wants to merge 1 commit intopingcap:masterfrom
Open
Relax benchmark test assertions to fix test failures on slower hardware#21ottok wants to merge 1 commit intopingcap:masterfrom
ottok wants to merge 1 commit intopingcap:masterfrom
Conversation
The benchmark tests were failing because they used hard-coded regex patterns that expected specific iteration counts and timing values. These expectations do not account for different hardware speeds or build environment load. Change the regex patterns to use generic digit placeholders (`\d+`) instead of specific numeric values. This allows the tests to verify the benchmark output formatting works correctly without being sensitive to actual performance characteristics of the build machine.
dveeden
approved these changes
Feb 27, 2026
|
This isn't a tiup dependency, right? |
Author
|
Correct, it is not a TiUP dependency |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The benchmark tests were failing in Debian in 2017 (this package has been in Debian for a long time!) and were disabled in https://salsa.debian.org/go-team/packages/golang-github-pingcap-check/-/commit/d0e1f2a213ad6a4546c331490d96c288556080e8
Tests failed because they used hard-coded regex patterns that expected specific iteration counts and timing values. These expectations do not account for different hardware speeds or build environment load.
The file
benchmark_test.godoes not have any inline comments explaining why the tests are done line this, nor does the commit 3819dcd from 2012 explain why it was done like this. Incidentally I did talk to the author Gustavo Niemeyer a few weeks ago on another topic, but I am not going to bother him now on this tiny detail and I elected to just allow the benchmark to return any numbers.With this change applied in Debian (https://salsa.debian.org/go-team/packages/golang-github-pingcap-check/-/commit/6456b7b5894de54f8337cb4560945bb0ad5124ea) I was able to enable the tests. This was the only test in the package, so I felt it makes sense to have it enabled even with the risk that some nuance with the result numbers is missed. At least it proves that the code runs and the rather old code built a functioning executable.
I leave it up to you as upstream to decide if you want to have this change, or if you want to submit it to your own upstream, or just ignore it, as it might be that this Go module isn't maintained anymore at all anyway (if that is the case, please put the GitHub repo in Archive mode to signify that it is no longer maintained).
Package status in Debian in case you are interested: https://tracker.debian.org/pkg/golang-github-pingcap-check
CC: @dveeden