Skip to content

chore(test): add tests to memory store #2226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 9, 2025
Merged

Conversation

JGAntunes
Copy link
Member

@JGAntunes JGAntunes commented Jun 2, 2025

What this PR does / why we need it:

Follow up to #2187

See:

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/124364/add-tests-to-memory-store

Does this PR require a test?

Does this PR require a release note?


Does this PR require documentation?

Copy link

github-actions bot commented Jun 2, 2025

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-58ccf60" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-58ccf60?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

Comment on lines +41 to +43
exitCode := int32(0)
osExit = func(code int) {
exitCode = code
atomic.StoreInt32(&exitCode, int32(code))
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed to change this for got test -race.

@JGAntunes JGAntunes requested review from emosbaugh and sgalsaleh June 2, 2025 14:28
Makefile Outdated
@@ -274,7 +274,7 @@ envtest:
.PHONY: unit-tests
unit-tests: envtest
KUBEBUILDER_ASSETS="$(shell ./operator/bin/setup-envtest use $(ENVTEST_K8S_VERSION) --bin-dir $(shell pwd)/operator/bin -p path)" \
go test -tags $(GO_BUILD_TAGS) -v ./pkg/... ./cmd/... ./api/... ./web/... ./pkg-new/...
go test -race -tags $(GO_BUILD_TAGS) -v ./pkg/... ./cmd/... ./api/... ./web/... ./pkg-new/...
Copy link
Member

Choose a reason for hiding this comment

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

how much time does this add to tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

On my machine:

time go test -count=1 -race -tags containers_image_openpgp,exclude_graphdriver_btrfs,exclude_graphdriver_devicemapper,exclude_graphdriver_overlay -v ./pkg/... ./cmd/... ./api/... ./web/... ./pkg-new/...
(...)
________________________________________________________
Executed in   28.01 secs    fish           external
   usr time   75.95 secs    0.06 millis   75.95 secs
   sys time   21.40 secs    1.70 millis   21.40 secs
time go test -count=1 -tags containers_image_openpgp,exclude_graphdriver_btrfs,exclude_graphdriver_devicemapper,exclude_graphdriver_overlay -v ./pkg/... ./cmd/... ./api/... ./web/... ./pkg-new/...
(...)
________________________________________________________
Executed in   23.82 secs    fish           external
   usr time   48.35 secs    0.07 millis   48.35 secs
   sys time   22.86 secs    2.39 millis   22.86 secs

If we want to look at CI:

Copy link
Member Author

Choose a reason for hiding this comment

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

About -race btw, seems like apple silicon's linker is currently throwing some warnings - golang/go#61229 (comment) - which according to that shouldn't affect the test execution though.

emosbaugh
emosbaugh previously approved these changes Jun 3, 2025
@JGAntunes JGAntunes force-pushed the chore/test-memory-store branch from 51a9e5c to 58ccf60 Compare June 9, 2025 18:06
Comment on lines +797 to +799
// Create a runtimeconfig to be used in the install process
rc := runtimeconfig.New(nil)

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to use the same runtime config for both manager and controller.

@emosbaugh
Copy link
Member

Fix for the test flake #2278

@emosbaugh emosbaugh enabled auto-merge (squash) June 9, 2025 21:56
@emosbaugh
Copy link
Member

Failure is unrelated. Merging.

@emosbaugh emosbaugh disabled auto-merge June 9, 2025 22:32
@emosbaugh emosbaugh merged commit 1345155 into main Jun 9, 2025
125 of 130 checks passed
@emosbaugh emosbaugh deleted the chore/test-memory-store branch June 9, 2025 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants