client_golang
client_golang copied to clipboard
ci: daggerize test and lint pipelines
:wave: coming from #1436
TL;DR:
- Install the
daggerCLI (https://docs.dagger.io/quickstart/cli) - Check out this PR and run:
dagger --src . call make-all --args "check_license test"
you can alternatively tell dagger to run the tests against this PR's branch without having to check out your code in your machine as follows:
dagger call -m github.com/prometheus/client_golang@pull/1534/merge --src https://github.com/prometheus/client_golang#pull/1534/merge \
make-all --args "check_license test"
This runs the same tests the current GHA pipeline runs here https://github.com/prometheus/client_golang/actions/runs/9419021570/workflow#L60 for all the supported_go_versions.txt file.
Hopefully this serves as a starting point to make this project's CI pipelines more reproducible and consistent across different environments :pray: .
Some considerations I took while daggerizing this pipeline:
- I've decided to "wrap" as much as I could in order to make the change the least disruptive. As you can see from the PR, I kept the same
makeUX the current repository has - I mostly focused on the
go.yml(https://github.com/prometheus/client_golang/actions/runs/9419021570/workflow) andgolangci-lint.yml(https://github.com/prometheus/client_golang/actions/runs/9419021582/workflow) workflows as those are mostly the ones that get executed for each PR and commit tomain. - I've noticed that the GHA workflow files seem to updated from the
prometheus/prometheusrepository. Given that I needed to modify them, I just did it in this repo as I'm unsure which would be the proper way to proceed here.
cc @kakkoyun
you can alternatively tell dagger to run the tests against this PR's branch without having to check out your code in your machine as follows:
dagger -m github.com/prometheus/client_golang@refs/pull/1534/merge call --src [email protected]:prometheus/client_golang#refs/pull/1534/merge make-all --args "check_license test"
For some reason that git URL failed for me, but this worked:
dagger call -m github.com/prometheus/client_golang@pull/1534/merge --src https://github.com/prometheus/client_golang#pull/1534/merge \
make-all --args "check_license test"
I get some test failures, is that expected @marcosnils ?
[...]
FAIL github.com/prometheus/client_golang/prometheus 16.423s
ok github.com/prometheus/client_golang/prometheus/collectors 0.022s
ok github.com/prometheus/client_golang/prometheus/graphite 0.026s
ok github.com/prometheus/client_golang/prometheus/internal 0.026s
ok github.com/prometheus/client_golang/prometheus/promauto 0.026s
ok github.com/prometheus/client_golang/prometheus/promhttp 0.386s
ok github.com/prometheus/client_golang/prometheus/push 0.023s
ok github.com/prometheus/client_golang/prometheus/testutil 0.026s
ok github.com/prometheus/client_golang/prometheus/testutil/diff 0.023s
ok github.com/prometheus/client_golang/prometheus/testutil/promlint 0.023s
FAIL
Stderr:
make: *** [Makefile.common:153: common-test] Error
For some reason that git URL failed for me, but this worked:
ok, strange.. both seem to work for me. I'll update the original URL to use yours in the original description :pray:
PR body does not contain a valid type of change. Please refer to CONTRIBUTING.md for more information.
I get some test failures, is that expected @marcosnils ?
just :eyes: at this failures and I have the theory that they're related to some flukes due to timing nits in tests: https://github.com/marcosnils/client_golang/blob/e56689d0f2d0bbff8be277c2e4a8dd71c89ce47a/prometheus/summary_test.go?plain=1#L389. Since make-all runs all go tests in parallel, looks like it's causing that tests to match incorrect assertions.
cc @kakkoyun WDYT?
@marcosnils Awesome! Thanks for the kickstart. I'll check this PR in-depth in the upcoming days. And see what we can improve.
hey @kakkoyun gentle ping here. LMK if you want me to rebase this PR if you have the time to :eyes: :pray:
@marcosnils, @ArthurSens, and I finally had some time to spare on this. Thanks again for the contribution.
@kakkoyun @ArthurSens I've addressed all the comments above. Please let me know if there's anything else that you'd prefer to be changed :pray:
I've also bumped the dagger version to the latest v0.12 and also rebased against main to resolve conflicts.
things (that means slowly migrating to dagger/main from Make right?
This would be the ideal. At the same time, since Dagger is still calling the same underlying make targets, IMO it's a "good enough" trade-off as an initial approach to see how Dagger feel :relaxed:
If we can fix the naming and add a comment to explain the intention, we are golden.
done! the comment was already there :open_hands: (https://github.com/prometheus/client_golang/pull/1534/files#diff-f18b612d33c74a552af8f75da09a36af695ef16d51ef7537bd1e7ef2d3d09685R53)