Skip to content

test: render sloth_version golden label with YAML-correct quoting - #826

Open
robbat2 wants to merge 1 commit into
slok:mainfrom
coreweave:rjohnson/upstream-numeric-version-fix
Open

robbat2 wants to merge 1 commit into
slok:mainfrom
coreweave:rjohnson/upstream-numeric-version-fix

Conversation

@robbat2

@robbat2 robbat2 commented Jun 7, 2026

Copy link
Copy Markdown

The Prometheus generate integration tests and the library use-case tests both substitute the build version (from git describe --tags --always) into shared golden .tpl files. When no tag is reachable that resolves to the bare short git SHA, which is all decimal digits ~3.7% of the time (e.g. 6950793). Sloth's YAML serializers quote such a value ("6950793") so it round-trips as a string, but the templates emitted it unquoted, so the assertion failed only on those commits.

Add a shared yamlValue template helper (testutils.GoldenTemplateFuncs) that marshals the version through yaml.v2 — matching both the Prometheus and k8s serializers — and use it for the sloth_version label from both test consumers. The comment line stays raw. The tests now pass for numeric SHAs, hex SHAs, and semver-style versions alike.

The Prometheus generate integration tests and the library use-case tests both
substitute the build version (from `git describe --tags --always`) into shared
golden .tpl files. When no tag is reachable that resolves to the bare short git
SHA, which is all decimal digits ~3.7% of the time (e.g. 6950793). Sloth's YAML
serializers quote such a value ("6950793") so it round-trips as a string, but
the templates emitted it unquoted, so the assertion failed only on those
commits.

Add a shared `yamlValue` template helper (testutils.GoldenTemplateFuncs) that
marshals the version through yaml.v2 — matching both the Prometheus and k8s
serializers — and use it for the sloth_version label from both test consumers.
The comment line stays raw. The tests now pass for numeric SHAs, hex SHAs, and
semver-style versions alike.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@robbat2
robbat2 requested a review from slok as a code owner June 7, 2026 15:12
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 15 days.

@github-actions github-actions Bot added the stale label Sep 6, 2026
@cxdy cxdy removed the stale label Sep 7, 2026

@cxdy cxdy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello again @robbat2, thanks for the contribution!

Just a couple nits, but looks real nice so far.

// so it is not parsed back as a number. Using this for the sloth_version label
// keeps the golden files correct regardless of whether the version happens to
// look numeric.
"yamlValue": func(v string) (string, error) {

@cxdy cxdy Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yamlValue is the whole fix, but nothing deterministically exercises "6950793""6950793" (quoted). test/integration/testutils has no tests, and the golden consumers only substitute whatever version this build happens to have. This HEAD is hex, CI is a shallow clone without tags, and the pkg/lib tests fall back to "dev" without ldflags, so CI on this PR never hits the case that motivated it. Someone could "simplify" yamlValue back to identity later and the flake comes back.

Can you add a small table-driven test that runs GoldenTemplateFuncs (even just {{ yamlValue . }}) for at least:

  • 6950793 (quoted)
  • things yaml.v2 will also quote, like 123e45 / 0123456 / yes
  • dev / abc1234 / v0.12.0 (plain)

Please put the input in the failure message (got != want).

"github.com/slok/sloth/internal/plugin"
"github.com/slok/sloth/pkg/common/model"
"github.com/slok/sloth/pkg/lib"
"github.com/slok/sloth/test/integration/testutils"

@cxdy cxdy Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's test-only so it's not leaking into the library, but unit tests in pkg/lib depending on the integration testutils package is a bit backwards. Sharing the FuncMap is the right idea, just a little odd that the shared helper lives under integration.

Not blocking, leaving it is fine if you don't want to bikeshed a new package for 15 lines. Would be cool to do now though so it's easier to expand on later!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants