Run tests in CI and fix failing tests. #1672
Conversation
jeschkies
force-pushed
the
karsten/ci-tests
branch
5 times, most recently
from
013873b to
5b3c575
Compare
| @@ -38,16 +38,16 @@ type cachingFactory interface { | |||
| Clear() | |||
| } | |||
|
|
|||
| func NewScraper(featureFlags []string) *scraper { //nolint:revive | |||
There was a problem hiding this comment.
Is it worth making scraper public as it's used only in main.go?
There was a problem hiding this comment.
The linter was complaining about the lint being present.
There was a problem hiding this comment.
I thought I saw a nolintlint false-positive bug recently.
There was a problem hiding this comment.
IIUC we could revert this specific change if it was just a false positive?
| originalValidationScheme := model.NameValidationScheme | ||
| model.NameValidationScheme = model.LegacyValidation | ||
| defer func() { | ||
| model.NameValidationScheme = originalValidationScheme | ||
| }() | ||
|
|
There was a problem hiding this comment.
I think this should raise attention to the breaking changes that added utf8 support:
- https://github.com/prometheus/common/releases/tag/v0.62.0
- https://github.com/prometheus/client_golang/releases/tag/v1.21.0
These two dependencies are now merged on master (unreleased), meaning that yace can now potentially export labels with utf8 characters. If the remote write destination does not support utf8, this would result in breaking behaviour right? I'm wondering if we should stick to the old validation scheme for some more time and add a feature flag to enable the new scheme.
There was a problem hiding this comment.
I agree. Could we do this in a follow up?
There was a problem hiding this comment.
Can we do the part where we set the legacy validation in this PR? Once pandora's box is opened, I guess it's worth merging a fully usable solution.
jeschkies
force-pushed
the
karsten/ci-tests
branch
from
5b3c575 to
71fa676
Compare
|
@jeschkies can you please update the commit message and PR description with more info on the change? |
jeschkies
force-pushed
the
karsten/ci-tests
branch
3 times, most recently
from
dc702f8 to
1f2882c
Compare
Signed-off-by: Karsten Jeschkies <[email protected]>
jeschkies
force-pushed
the
karsten/ci-tests
branch
from
1f2882c to
c19e49b
Compare
The unit tests weren't running with each PR. This change adds a
test_gostep to the GitHub actions workflow.Some tests were failing. This is due to a breaking change in Prometheus supporting UTF-8 characters.
NameValidationSchemeis set toLegacyValidationglobally to ensure the behaviour or former releases.