Merged
Conversation
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
…update method names and fix class references Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
There was a problem hiding this comment.
Pull request overview
Adds new integration coverage for DNS resolution and TLS connectivity variants in the C# client, and refactors shared test utilities to reduce duplication.
Changes:
- Introduces DNS/host-based integration tests and expands TLS test coverage.
- Centralizes shared theory data (
ClusterMode) and updates PubSub tests to consume it. - Enhances test
Serverutility with public addresses and certificate accessors; renames client creation APIs to async-style.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| valkey-glide | Updates submodule pointer to bring in upstream changes needed by the new tests/utilities. |
| tests/Valkey.Glide.TestUtils/Server.cs | Exposes server addresses/certs for tests and renames client creation methods to *Async. |
| tests/Valkey.Glide.TestUtils/Data.cs | Adds shared ClusterMode theory data for reuse across test suites. |
| tests/Valkey.Glide.IntegrationTests/TlsTests.cs | Refactors TLS tests/fixture and adds new certificate handling pathways. |
| tests/Valkey.Glide.IntegrationTests/PubSubUtils.cs | Switches PubSub theory generation to shared Data.ClusterMode. |
| tests/Valkey.Glide.IntegrationTests/PubSubQueueTests.cs | Updates MemberData to shared cluster-mode data. |
| tests/Valkey.Glide.IntegrationTests/PubSubIntrospectionTests.cs | Updates MemberData and server client creation calls to async naming. |
| tests/Valkey.Glide.IntegrationTests/PubSubEdgeCases.cs | Updates MemberData to shared cluster-mode data. |
| tests/Valkey.Glide.IntegrationTests/PubSubCoexistenceTests.cs | Updates MemberData to shared cluster-mode data. |
| tests/Valkey.Glide.IntegrationTests/PubSubCallbackTests.cs | Updates MemberData to shared cluster-mode data. |
| tests/Valkey.Glide.IntegrationTests/PubSubBasicTests.cs | Updates MemberData to shared cluster-mode data. |
| tests/Valkey.Glide.IntegrationTests/ISubscriberCompatibilityTests.cs | Updates MemberData to shared cluster-mode data. |
| tests/Valkey.Glide.IntegrationTests/DnsTests.cs | Adds new host/DNS-oriented connectivity tests (cluster/standalone, TLS/non-TLS, IPv4/IPv6). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: currantw <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
7 tasks
Signed-off-by: currantw <[email protected]>
This was referenced Mar 2, 2026
… into currantw/dns_csharp_tests Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
jeremyprime
reviewed
Mar 19, 2026
tests/Valkey.Glide.IntegrationTests/PubSubIntrospectionTests.cs
Outdated
Show resolved
Hide resolved
jeremyprime
reviewed
Mar 19, 2026
jeremyprime
reviewed
Mar 19, 2026
jeremyprime
reviewed
Mar 19, 2026
xShinnRyuu
reviewed
Mar 19, 2026
… into currantw/dns_csharp_tests
xShinnRyuu
reviewed
Mar 19, 2026
tests/Valkey.Glide.IntegrationTests/PubSubIntrospectionTests.cs
Outdated
Show resolved
Hide resolved
…el`. Update some `PubSubIntrospectionTests` tests to use `Equivalent` instead of `Contains`. Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
…nitialized if tests are disabled. Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
Signed-off-by: currantw <[email protected]>
jeremyprime
approved these changes
Mar 20, 2026
xShinnRyuu
approved these changes
Mar 20, 2026
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
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.
Summary
Add integration tests for DNS resolution and TLS IPv4/IPv6 connectivity in the C# client.
Issue Link
⚪ None
Features and Behaviour Changes
Add new DNS and TLS tests, and centralize common test data and constants.
See more details below.
Implementation
New Tests
DnsTestsclass: DNS resolution tests with and without TLS for both cluster and standalone:TlsTestsenhancements: TLS connectivity tests with IPv4 and IPv6 addresses:127.0.0.1).::1).ClusterClientTestsandStandaloneClientTestsenhancements: connectivity tests with IPv4 and IPv6 addresses (without TLS):127.0.0.1).::1).DNS tests require hostname mappings and the environment variable
VALKEY_GLIDE_DNS_TESTS_ENABLEDto be set.Test Infrastructure Improvements
TestUtils.Constantsclass: Centralized shared test constants to eliminate duplication across test files:HostnameTls- Hostname included in certificate SANHostnameNoTls- Hostname not in certificate SANIpv4Address- IPv4 loopback addressIpv6Address- IPv6 loopback addressTestUtils.Dataclass: Centralized common test data to eliminate duplication across test files, withClusterModeandIpAddresses.Serverclass enhancements:Addressesproperty public for test access.CertificatePathandCertificateDataproperties (reduce dependence onServerManagerand hide the fact that all servers share the same certificates).Test refactoring: Updated all PubSub and other integration tests to use centralized
Data.ClusterModeinstead of local definitions, improving consistency and maintainability.Limitations
DNS tests require host file entries mapping
valkey.glide.test.tls.comandvalkey.glide.test.no_tls.comto127.0.0.1and::1. These must be configured manually for local development (automatically configured in CI).Testing
See New Tests above.
Related Issues
Checklist
This Pull Request is related to one issue.CHANGELOG.md,README.md,DEVELOPER.md, and other documentation files are updated.mainor releasemain, squash otherwise.