adding dynamic dns tests#8415

Draft

danlavu wants to merge 1 commit intoSSSD:masterfrom

danlavu:tests-dyndns

danlavu commented Feb 2, 2026

No description provided.

danlavu marked this pull request as draft

February 2, 2026 16:12

gemini-code-assist bot reviewed

View reviewed changes

gemini-code-assist bot left a comment

Code Review

This pull request introduces a new test file for SSSD's dynamic DNS functionality, test_dyndns.py. It adds an initial test case and stubs for several others. The review identified some critical issues: there are two pairs of duplicated test function definitions, which will cause errors. I've suggested removing these duplicates. Additionally, the implemented test case uses a fixed time.sleep(), which is a common source of test flakiness. I've recommended replacing it with a polling mechanism to improve test reliability and efficiency.

src/tests/system/tests/test_dyndns.py

Comment on lines +75 to +89

+              def test_dyndns__update_creates_forward_ipv4_records(client: Client, provider: GenericProvider):
+                  """
+                  :title: Dynamic DNS updates IPV4 address only
+                  :description: SSSD should create records for network addresses that are on the client
+                  :setup:
+. Remove IPV6 address from ethernet interface
+. Create PTR zone for default network
+. Start SSSD
+                  :steps:
+. Check forward zone for client's A record
+                  :expectedresults:
+. Client A record exists and is the only client record in the zone file
+                  :customerscenario: True
+                  """

gemini-code-assist bot Feb 2, 2026

The test function test_dyndns__update_creates_forward_ipv4_records is defined twice. This duplicate definition on lines 76-89 should be removed, as it overwrites the original definition on lines 60-73. Python does not allow duplicate function names in the same module.

src/tests/system/tests/test_dyndns.py

Comment on lines +111 to +129

+              def test_dyndns__updates_all_forward_records(client: Client, provider: GenericProvider):
+                  """
+                  :title: Dynamic DNS updates AAAA records on all interfaces
+                  :description: SSSD should update all records if the IP changes
+                  :setup:
+. Create PTR zone
+.  A/AAAA/PTR records that DO NOT match the client's IP Address
+. Start SSSD
+                  :steps:
+. Check forward zone for client's A record
+. Check forward zone for client's AAAA record
+. Check reverse zone for client's pointer record
+                  :expectedresults:
+. Client A record exists in the zone file
+. Client AAAA record exists in the zone file
+. Client PTR record exists and points to the latest IP
+                  :customerscenario: True
+                  """

gemini-code-assist bot Feb 2, 2026

The test function test_dyndns__updates_all_forward_records is defined twice. This duplicate definition on lines 112-129 should be removed, as it overwrites the original definition on lines 92-109. Python does not allow duplicate function names in the same module.

src/tests/system/tests/test_dyndns.py Outdated

Comment on lines 54 to 57

+                  time.sleep(15)
+                  assert client.net.nslookup([f"client.{provider.domain}", provider.server]) == 0, f"Host client.{provider.domain} was not found!"
+                  assert client.net.nslookup([client.net.ip().address, provider.server]) == 0, f"Host address {client.net.ip().address} was not found!"

gemini-code-assist bot Feb 2, 2026

Using a fixed time.sleep(15) can lead to flaky tests. If the DNS update takes longer than 15 seconds, the test will fail. Conversely, if the update is much faster, the test waits unnecessarily, slowing down the test suite. A better approach is to poll for the desired state with a timeout. This makes the test more robust and potentially faster.

    # Poll for DNS records to appear
    hostname_found = False
    ip_found = False
    for _ in range(30):  # Poll for up to 30 seconds
        if not hostname_found and client.net.nslookup([f"client.{provider.domain}", provider.server]) == 0:
            hostname_found = True
        if not ip_found and client.net.nslookup([client.net.ip().address, provider.server]) == 0:
            ip_found = True
        if hostname_found and ip_found:
            break
        time.sleep(1)

    assert hostname_found, f"Host client.{provider.domain} was not found!"
    assert ip_found, f"Host address {client.net.ip().address} was not found!"

github-advanced-security bot found potential problems

View reviewed changes

src/tests/system/tests/test_dyndns.py Fixed Show fixed Hide fixed

src/tests/system/tests/test_dyndns.py

		"""


		def test_dyndns__updates_all_forward_records(client: Client, provider: GenericProvider):

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning test

This assignment to 'test_dyndns__updates_all_forward_records' is unnecessary as it is

before this value is used.

src/tests/system/tests/test_dyndns.py Fixed Show fixed Hide fixed

joakim-tjernlund commented Feb 3, 2026

Would be great with a test that simulates a working LAN connection and then LAN goes offline and WiFi comes up.
In my experience it can take minutes to update DNS and RDNS updates leaves the old LAN entry in RDNS


          adding dynamic dns tests

b4dd7cc

danlavu force-pushed the tests-dyndns branch from 544cee8 to b4dd7cc Compare

February 13, 2026 15:37

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

Labels

None yet