chore(ci): speed-up & coveralls

[mloiseleur]

What does it do ?

1. Switch CI to use coveralls
2. Rename job name from "Build" to "Test", since it calls make test
3. Upgrade resources, following Allocate more resources on ExternalDNS jobs kubernetes/test-infra#35591

Motivation

1. Fixes ci: add check on test coverage regression #5854
2. Speed-up CI

More

• Yes, this PR title follows Conventional Commits
• Yes, I added unit tests
• Yes, I updated end user documentation accordingly

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coveralls]

Pull Request Test Coverage Report for Build 18088343560

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.005%) to 78.532%

---

Totals
Change from base Build 18019814512:	0.005%
Covered Lines:	15766
Relevant Lines:	20076

---

💛 - Coveralls

[mloiseleur]

@ivankatliarchuk @AndrewCharlesHay @TobyTheHutt I managed to get the PR comment and the status in the CI.

FTM, it's configured like this on threshold:

Wdyt ? Anything missing ?

[TobyTheHutt]

Maybe I'm missing something important. I don't see how Coveralls is acceptable in this implementation, since to me, the conflict mentioned in #5854 is still valid and open.

But if it's just about the minimum coverage, I can propose a Makefile extension like the following:

go-test:
	GOCACHE=$(GOCACHE) go test -race -covermode=$(COVERMODE) -coverprofile=$(COVERFILE) ./...
	@GOCACHE=$(GOCACHE) go tool cover -func=$(COVERFILE) > coverage.summary
	@awk '/^total:/ { if ($$3+0 < $(COVER_THRESHOLD)) { printf "Coverage %.1f%% is below threshold $(COVER_THRESHOLD)%%\n", $$3+0; exit 1 } }' coverage.summary
	@echo "Coverage check passed (>= $(COVER_THRESHOLD)% )"

COVER_THRESHOLD is variable containing a numerical value like 75.0, which is then matched against the coverage report.
The advantage: Every contributor can make their changes PR-ready before committing. Plus, no additional integrations are needed.

If we wanted, we could even split the coverage check on individual packages.

[ivankatliarchuk]

We need to enable an app as well on repo level for coveralls to works in the way it should.

[ivankatliarchuk]

/lgtm

[mloiseleur]

@TobyTheHutt I agree with you that it's good to have a local command in Makefile. I added a go-test entry in Makefile with d55ebe7.

By taking a look at the coverage summary, we can see it's not that easy as putting a threshold on each line of the summary. For instance, we can see that some are at 0%, like those ones:

sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:13:		DeepCopyInto						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:22:		DeepCopy						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:32:		DeepCopyObject						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:40:		DeepCopyInto						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:54:		DeepCopy						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:64:		DeepCopyObject						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:72:		DeepCopyInto						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:88:		DeepCopy						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:98:		DeepCopyInto						0.0%
sigs.k8s.io/external-dns/apis/v1alpha1/zz_generated.deepcopy.go:103:		DeepCopy						0.0%

And it's expected, it's on generated code. We have also many not-covered func:

$ awk '{ if ($3 == "0.0%") { printf "%s: %s\n", $1, $3 } }' < coverage.summary  | wc -l
233

So far, I fail to see how we can set easily a minimum threshold.

ExternalDNS current coverage is around 78%, so I set 75% as a start point.
Theoretically, to fully address #5854, we can set 100% on "Patch coverage threshold".
In practice, it may also block useful PRs with code that do not need to be that strict.

IMHO, we should take care of not making this as a blocker and go step by step.
In few weeks, we can increase this "patch coverage threshold" based on what we observed on PRs.

@TobyTheHutt Does that address your concerns about this topic ? Or is your concern is more about using an online solution like coveralls ?

[TobyTheHutt]

@mloiseleur Thank you for the feedback! I agree that this discussion should not block the PR. My concern has been properly addressed and I have no objections to this PR.

[mloiseleur]

@AndrewCharlesHay I'll need your review on this PR and we should be good to go.

[AndrewCharlesHay]

/lgtm

[AndrewCharlesHay]

Thanks!

[mloiseleur]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mloiseleur]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment