Skip to content

Conversation

underwoo16
Copy link

No description provided.

@Copilot Copilot AI review requested due to automatic review settings July 29, 2025 18:57
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds a test for DNS hostname lookup failure scenarios in the Memcached client. The test verifies that the client properly handles NXDOMAIN responses and server ejection behavior when configured with auto_eject_hosts.

  • Added test_dns_nxdomain_server method to test DNS failure handling
  • Tests server ejection and recovery behavior when DNS lookups fail
  • Verifies failover to working servers when one server has DNS issues

assert_raise(Memcached::HostnameLookupFailure) { cache.get(key2, @value) }

# Hit second server and pass the limit
key2 = 'test_missing_server'
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The variable key2 is redeclared multiple times with the same value. Consider declaring it once at the beginning of the test method to avoid duplication.

Suggested change
key2 = 'test_missing_server'
key2 = 'test_missing_server'
# Hit second server up to the server_failure_limit
assert_raise(Memcached::HostnameLookupFailure) { cache.set(key2, @value) }
assert_raise(Memcached::HostnameLookupFailure) { cache.get(key2, @value) }
# Hit second server and pass the limit

Copilot uses AI. Check for mistakes.

# Hit second server up to the server_failure_limit
key2 = 'test_missing_server'
assert_raise(Memcached::HostnameLookupFailure) { cache.set(key2, @value) }
assert_raise(Memcached::HostnameLookupFailure) { cache.get(key2, @value) }
Copy link
Preview

Copilot AI Jul 29, 2025

Choose a reason for hiding this comment

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

The get method typically doesn't accept a second parameter for the value. This should likely be cache.get(key2) without the @value parameter.

Suggested change
assert_raise(Memcached::HostnameLookupFailure) { cache.get(key2, @value) }
assert_raise(Memcached::HostnameLookupFailure) { cache.get(key2) }

Copilot uses AI. Check for mistakes.

@underwoo16 underwoo16 closed this Jul 29, 2025
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.

1 participant