chore(tests): introduce testcontainers into integration tests as a partial replacement for chainsaw #2083
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Baarsgaard
changed the title
Baarsgaard
force-pushed
the
chore_e2e_to_testcontainers
branch
from
7ec5da8 to
74a20b6
Compare
Baarsgaard
requested review from
nissessenap,
ishanjainn,
theSuess,
hubeadmin and
pb82
as code owners
Baarsgaard
force-pushed
the
chore_e2e_to_testcontainers
branch
2 times, most recently
from
55d375e to
1d6d6bf
Compare
Baarsgaard
changed the title
Baarsgaard
changed the title
Baarsgaard
changed the title
weisdd
approved these changes
Jul 18, 2025
There was a problem hiding this comment.
I think there's potential for code simplification and improved consistency, but I guess it's OK to do that later as reducing dependency on chainsaw and ability to test positive conditions are more important, so I don't want to block you.
I've made a few minor suggestions though on what you might want to improve right away.
weisdd
changed the title
weisdd
changed the title
Baarsgaard
force-pushed
the
chore_e2e_to_testcontainers
branch
2 times, most recently
from
c106c41 to
07050b0
Compare
|
Agreed, There's a likely a lot that could be done for consistency, naming, and similar. For example the added file |
weisdd
requested changes
Jul 21, 2025
Baarsgaard
force-pushed
the
chore_e2e_to_testcontainers
branch
from
07050b0 to
8327f58
Compare
Baarsgaard
changed the base branch from
master
to
fix_external_grafana_config_credentials_fallback
Co-authored-by: Igor Beliakov <46579601+weisdd@users.noreply.github.com>
Baarsgaard
force-pushed
the
chore_e2e_to_testcontainers
branch
from
8327f58 to
a6bf4d8
Compare
Base automatically changed from
fix_external_grafana_config_credentials_fallback
to
master
July 25, 2025 15:32
weisdd
approved these changes
Jul 25, 2025
|
I've fixed the formatting issue, so now we should be good to merge the PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a continued effort to improve CI stability, This PR introduces an External Grafana instance to Integration tests via testcontainers.
I mentioned wanting an cluster external grafana instance when running our Chainsaw E2E tests, but realised most of the benefits from that also applied to Integration tests.
Tp that end, the forced folder deletion and newly added datasource substitution tests have been migrated into Integration tests.
As unit tests are fast to run compared to integration tests, I added support for
-shortviamake test-short.When used, integration tests(Ginkgo) are skipped.
This allows significantly faster iteration when writing unit tests.
Refactored
GetScopedMatchingInstancesintegration test with new variables and more from the suite.Once merged, this allows testing positive Conditions like
Synchronizedas evident from the new tests.Coverage of controllers has gone from
35.9%to42.2%