Skip to content

Add DNS, TLS, and IP address tests#157

Open
currantw wants to merge 15 commits intomainfrom
currantw/dns_php_tests
Open

Add DNS, TLS, and IP address tests#157
currantw wants to merge 15 commits intomainfrom
currantw/dns_php_tests

Conversation

@currantw
Copy link
Copy Markdown
Collaborator

@currantw currantw commented Mar 3, 2026

Summary

Add integration tests for

  • DNS resolution (with and without TLS)
  • IPv4/IPv6 connectivity (with and without TLS)

for the PHP Valkey GLIDE client.

Issue link

⚪ None

Features / Behaviour Changes

Add new DNS and IP address connectivity tests. See more details below.

Implementation

New Tests

ValkeyGlideDnsTest.php: DNS resolution tests with and without TLS for standalone:

  • Non-TLS connection succeeds with valid hostname.
  • Non-TLS connection fails with invalid hostname.
  • TLS connection succeeds with hostname present in certificate SAN.
  • TLS connection fails with hostname not in certificate SAN.

ValkeyGlideClusterDnsTest.php: DNS resolution tests with and without TLS for cluster:

  • Non-TLS connection succeeds with valid hostname.
  • Non-TLS connection fails with invalid hostname.
  • TLS connection succeeds with hostname present in certificate SAN.
  • TLS connection fails with hostname not in certificate SAN.

ValkeyGlideTlsTest.php: Comprehensive TLS tests for standalone including IP address connectivity:

  • TLS connection succeeds with IPv4 address (127.0.0.1).
  • TLS connection succeeds with IPv6 address (::1).
  • Certificate validation tests (empty, invalid, multiple certificates).

ValkeyGlideClusterTlsTest.php: Comprehensive TLS tests for cluster including IP address connectivity:

  • TLS connection succeeds with IPv4 address (127.0.0.1).
  • TLS connection succeeds with IPv6 address (::1).
  • Certificate validation tests (empty, invalid, multiple certificates).

ValkeyGlideTest.php and ValkeyGlideClusterTest.php enhancements: Add non-TLS IP address connectivity tests:

  • Connection succeeds with IPv4 address (127.0.0.1).
  • Connection succeeds with IPv6 address (::1).

DNS tests require hostname mappings and the environment variable VALKEY_GLIDE_DNS_TESTS_ENABLED to be set.

Test Infrastructure Improvements

  • Add test utility helpers in ValkeyGlideBaseTest.php: assertConnected(), skipIfTlsDisabled(), skipIfTlsEnabled(), getCaCertificate(), and skipIfDnsNotEnabled().
  • Add assertThrows() helper to TestSuite.php for exception testing.
  • Add constants for hostnames and IP addresses to ValkeyGlideBaseTest.php.
  • Reorganized TLS tests from main test files into dedicated TLS test files.
  • Updated server configuration scripts to bind to both IPv4 and IPv6.
  • Added stop-servers.sh script for cleanup.

Limitations

DNS tests require host file entries mapping valkey.glide.test.tls.com and valkey.glide.test.no_tls.com to 127.0.0.1 and ::1. These must be configured manually for local development (automatically configured in CI).

Testing

See New Tests above.

Related Issues

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Linters have been run and pass.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

currantw added 15 commits March 2, 2026 11:33
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
…and ::1 (IPv6).

Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
Signed-off-by: currantw <taylor.curran@improving.com>
@currantw currantw self-assigned this Mar 3, 2026
*/
protected function skipIfDnsNotEnabled(): void
{
if (!getenv('VALKEY_GLIDE_DNS_TESTS_ENABLED')) {
Copy link
Copy Markdown
Collaborator

@prateek-kumar-improving prateek-kumar-improving Mar 3, 2026

Choose a reason for hiding this comment

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

Shouldn't we always run the tests? Don't see any situation why we would skip those and not want them to run. We don't have this we any other test types.
If we want to have this functionality, could you please document this in a comment under which conditions will it be useful to skip these tests?


/* Define to 1 if the PHP extension 'valkey_glide' is built as a dynamic
module. */
/* Whether to build valkey_glide as dynamic module */
Copy link
Copy Markdown
Collaborator

@prateek-kumar-improving prateek-kumar-improving Mar 3, 2026

Choose a reason for hiding this comment

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

Saw your commit message that this file is auto-generated. Is it auto-generated on local and needs to to be commited?

@@ -0,0 +1,102 @@
<?php

defined('VALKEY_GLIDE_PHP_TESTRUN') or die("Use TestValkeyGlide.php to run tests!\n");
Copy link
Copy Markdown
Collaborator

@prateek-kumar-improving prateek-kumar-improving Mar 3, 2026

Choose a reason for hiding this comment

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

Are these new tests getting picked on CI? You might need to add the following to TestValkeyGlide.php:

1. Add require_once __DIR__ . "/ValkeyGlideDnsTest.php";
2. Entries to $valid_classes array: 'valkeyglidedns' => 'ValkeyGlideDnsTest',
3. $default_classes .= 'valkeyglidetls,valkeyglideclustertls,valkeyglidedns,valkeyglideclusterdns';

Can you check if it is required in your case and check if the tests are actually running?

../valkey-glide/utils/cluster_manager.py --auth dummy_password stop --prefix auth-cluster || true
rm -rf ../valkey-glide/utils/clusters/auth-* 2>/dev/null || true

echo "✅ All Valkey servers stopped and cleaned up"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please remove the check mark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants