-
Notifications
You must be signed in to change notification settings - Fork 805
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
Migrate Redis tests to Testcontainers #2345
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks VERY clean and simple, great work @Alirexaa !
Which OS have you used it to test it locally?
What is needed to run it on Windows? WSL 2?
Do you think that testing other health checks that don't have a dedicated Testcontainers
package (like Testcontainers.Redis
here) is also going to be that simple?
test/HealthChecks.Redis.Tests/Functional/RedisHealthCheckTests.cs
Outdated
Show resolved
Hide resolved
Windows 10 with docker desktop. WSL 2 is necessary. The tests must be run on a machine with Docker already installed.
Most of them yes. Some of them may need more work to do. @adamsitnik, There are some examples you can take a look at. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you very much @Alirexaa !
What this PR does / why we need it:
This pull request includes several changes to integrate Testcontainers for Redis in the health checks testing suite. The most important changes include adding a new fixture for Redis containers, updating test cases to use the fixture, and modifying configuration files to include the new dependencies.
Integration of Testcontainers for Redis:
test/HealthChecks.Redis.Tests/RedisContainerFixture.cs
: Added a new fixture classRedisContainerFixture
to manage the lifecycle of a Redis container using Testcontainers.test/HealthChecks.Redis.Tests/RedisContainerImageTags.cs
: Added a static classRedisContainerImageTags
to define constants for the Redis container image tags.Updates to test cases:
test/HealthChecks.Redis.Tests/Functional/RedisHealthCheckTests.cs
: Modified test cases to use theRedisContainerFixture
for obtaining the Redis connection string instead of hardcoding it. [1] [2] [3] [4]Configuration changes:
.github/workflows/healthchecks_redis_ci.yml
: Removed the Redis service configuration from the CI workflow.Directory.Packages.props
: Added theTestcontainers.Redis
package version.build/versions.props
: Defined theTestcontainersVersion
property.test/HealthChecks.Redis.Tests/HealthChecks.Redis.Tests.csproj
: Added a package reference forTestcontainers.Redis
.Which issue(s) this PR fixes:
Contributes to #2335
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Please make sure you've completed the relevant tasks for this PR, out of the following list: