Add IPv6 availability check to skip tests when unavailable #2674

diego-ciciani01 · 2025-10-02T12:53:01Z

Problem e test suite fails when IPv6 is not available on the system with the error:

The test suite fails when IPv6 is not available on the system

Solution

Add is_ipv6_available() to detect IPv6 support
Add VALLKEY_REQUIRE_IPV6 env var (inside a 'if' condition) to force IPv6 tests in CI, implement in the pipeline the export and volorization of env var
Skip IPv6 tests automatically when IPv6 is not available
Wrapped IPv6 tests in tests/unit/introspection.tcl to skip when IPv6 is unavailable

Testing

Without IPv6: test are skipped automatically
With IPv6: test run normally
With VALKEY_REQUIRE_IPV6=1: tests always run (fails if IPv6 unavailable)
Fixes [BUG] make test should detect ipv6 support #2643

zuiderkwast

Good job. However, I think we should do it using the existing mechanism to skip tests based on the existing tagging.

The tests/test_helper.tcl is the entry point that checks command line arguments. Here, we can add "ipv6" to ::denytags if IPv6 is not available and not required.

zuiderkwast · 2025-10-06T10:37:17Z

tests/support/util.tcl

+    if {[catch {exec ifconfig lo0} result] == 0} {
+        if {[string match "*inet6*::1*" $result]} {
+            return 1
+        }
+    }
+
+    if {[catch {exec netstat -an} result] == 0} {
+        if {[string match "*::1*" $result]} {
+            return 1
+        }
+    }
+
+    if {[catch {
+        set sock [socket -async ::1 22]
+        close $sock
+        return 1
+    }]} {
+        return 0
+    }


Why do we need three ways? Can we use only the 3rd way to detect this and save the result in some global variable to avoid doing it multiple times?

I agree with you, multiple checks can seem a bit repetitive, but during development, the third test on my macbook never worked (I'll leave a test I did in the comments, but I didn't add it to the PR). I also made sure that IPv6 was enabled, so as a workaround I also did the first two checks.

I currently have version 9 of tcl.

here the the did

proc is_ipv6_available {} { if {[catch { set sock [socket -async ::1 0] close $sock } err]} { puts "ipv6 check failed: $err" return 0 } return 1 }

and i leave the result of the running test

Ah, you're testing to connect to port 22 = the SSH port. It is not always opened. We can not even assume that SSH is installed.

I think we can't assume that any specific port is opened. What we need to do is to create a server socket on a free port chosen by the OS. To do this, use port 0, a command like socket -server ::1 0. Read details under SERVER SOCKETS on the page https://www.tcl-lang.org/man/tcl/TclCmd/socket.html#M9

This is what I get on my system where IPv6 is available:

$ tclsh % set s [socket -server ::1 0] sockaaab2529cdd0 % chan configure $s -sockname 0.0.0.0 0.0.0.0 33953 :: :: 33953

Do the tests work with TCL 9? Do you get some error when running all the tests?

From what I remember, TCL 9 didn't work and I have opened #1673 to fix that.

Try this:

Suggested change

if {[catch {exec ifconfig lo0} result] == 0} {

if {[string match "*inet6*::1*" $result]} {

return 1

}

}

if {[catch {exec netstat -an} result] == 0} {

if {[string match "*::1*" $result]} {

return 1

}

}

if {[catch {

set sock [socket -async ::1 22]

close $sock

return 1

}]} {

return 0

}

if {[catch {

set sock [socket -server ::1 0]

close $sock

return 1

}]} {

return 0

}

I don't know how TCL behaves, but if set sock [socket -server ::1 0] never fails, even if IPv6 is not available, then we maybe also need to try to connect as a client to this server socket.

Something like this:

if {[catch { set server [socket -server ::1 0] set port [lindex [chan configure $server -sockname] 5] set client [socket ::1 $port] close $server close $client return 1 }]} { return 0 }

I solved the problem. As you said, version 9 of TCL doesn't work very well. I switched to 8.6, and now the socket opens correctly.

I will test also the client - server connect

why did you set set port [lindex [chan configure $server -sockname] 5] ?

i thinks that the correct value should be 2, because this position returns the port

I didn't find the docs for this but I tested locally in tclsh and I got the port on two places:

% chan configure $sock -sockname 0.0.0.0 0.0.0.0 35783 :: :: 35783

It looks like IPv4 IPv4 port IPv6 IPv6 port. The last port maybe is for IPv6. That's what I was thinking. I don't know if it's correct. It's better to find the proper documentation for this.

aah it make sense, so i found this doc ->https://core.tcl-lang.org/tips/doc/trunk/tip/162.md
which explains exactly your test.

So i also added few commits with the changes that you told me

tests/unit/introspection.tcl

tests/support/util.tcl

diego-ciciani01 · 2025-10-07T09:58:50Z

hi @zuiderkwast, thank you very much for your feedback. If you need any further clarification, please let me know. Your advice is clear to me, please also let me know what you think about the IPv6 socket issue, after which I will modify the PR with your suggestions.

codecov · 2025-10-07T16:20:53Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.55%. Comparing base (8182f4a) to head (d77f90d).

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2674      +/-   ##
============================================
- Coverage     72.58%   72.55%   -0.03%     
============================================
  Files           128      128              
  Lines         71277    71277              
============================================
- Hits          51736    51718      -18     
- Misses        19541    19559      +18

see 23 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Add is_ipv6_available() to detect IPv6 support - Add VALLKEY_REQUIRE_IPV6 env var (inside a 'if' condition) to force IPv6 tests in CI, implement in the pipeline the export and volorization of env var - Skip IPv6 tests automatically when IPv6 is not available - BUG valkey-io#2643 Signed-off-by: diego-ciciani01 <[email protected]>

Moves utility functions previously defined in to the central test helper file, , for better organization and consistency across the testing framework. Additionally, this commit introduces a new tag-based logic to conditionally run tests based on the availability of IPv6 support on the system. Tests requiring IPv6 connectivity are now tagged accordingly, ensuring that they are only executed when the necessary network configuration is present. Signed-off-by: diego-ciciani01 <[email protected]>

zuiderkwast

Great! This is getting closer. I have some more ideas though after reading code and documentation. (None of us is a Tcl expert.)

zuiderkwast · 2025-10-09T19:44:16Z

tests/unit/introspection.tcl

        assert_match *client-ip* $filtered
    } {}

+


Don't add these extra blank lines. Keep the diff small and avoid this kind of whitespace changes.

Suggested change

zuiderkwast · 2025-10-09T19:44:23Z

tests/unit/introspection.tcl

Suggested change

zuiderkwast · 2025-10-09T19:44:32Z

tests/unit/introspection.tcl

        assert_error "*I/O error*" {$c1 ping}
    } {}

+


Suggested change

zuiderkwast · 2025-10-09T19:45:41Z

tests/unit/introspection.tcl

            $c close
        }
    }
+


Suggested change

zuiderkwast · 2025-10-09T19:45:57Z

tests/unit/introspection.tcl

        r client list not-ip $not_ip not-ip $not_ip
    } {}
-
+   


Suggested change

zuiderkwast · 2025-10-09T19:52:05Z

tests/unit/introspection.tcl

+            regexp {addr=\[([a-fA-F0-9:]+)\]:\d+} $client_info -> ipv6only
+            set filtered [$c client list not-ip $ipv6only]
+            assert_no_match *client-ipv6* $filtered
+
            regexp {addr=\[([a-fA-F0-9:]+)\]:\d+} $client_info -> ipv6only
            set filtered [$c client list not-ip "1.2.3.4" not-ip $ipv6only]
            assert_no_match *client-ipv6* $filtered


What's the purpose of adding this additional check?

It is very similar to teh check below, but is there a risk that matching either "1.2.3.4" or the client's IPv6 address will accidentally match because the client's IPv4 address happens to be 1.2.3.4?

The duplicated lines probably appeared after I merged or rebased this branch with main while the PR was already open.
I will remove this typo

zuiderkwast · 2025-10-09T19:53:09Z

tests/test_helper.tcl

+#IPv6 detection utilities
+#for this detaction: the socket connection on ::1 address


Add a space between the # and the text, for better readability.

Suggested change

#IPv6 detection utilities

#for this detaction: the socket connection on ::1 address

# IPv6 detection utilities

# for this detaction: the socket connection on ::1 address

zuiderkwast · 2025-10-09T22:28:17Z

tests/test_helper.tcl

+        if {[catch {
+            set server [socket -server ::1 0]
+            set port [lindex [chan configure $server -sockname] 5]


I'm reading the documentation (https://www.tcl-lang.org/man/tcl/TclCmd/socket.html) for server sockets again. The server sockets, the syntax is

socket -server command ?options? port

The IP address is not specified here. The command is the code to run when a client connects to it. The documentation says:

For each connection Tcl will create a new channel that may be used to communicate with the client. Tcl then invokes command (properly a command prefix list, see the EXAMPLES below) with three additional arguments: the name of the new channel, the address, in network address notation, of the client's host, and the client's port number.

Anyway, it seems to be OK to not have a correct command here, but I guess we should not write ::1 here because it's not an IP address at all. Better just use dummy or as the command (which will never be called anyway if the server doesn't start an event loop using vwait).

Also, if we use the option -myaddr ::1, then the server will only listen on the IPv6 address and the chan configure will only return 3 elements, where the last one is the port. That's also documented here under CONFIGURATION OPTIONS, -sockname:

For server sockets this option returns a list of a multiple of three elements each group of which have the same meaning as described above. The list contains more than one group when the server socket was created without -myaddr or with the argument to -myaddr being a domain name that resolves multiple IP addresses that are local to the invoking host.

One more thing, the indentation here looks odd. Use 4 spaces for each level.

To sum up:

Suggested change

if {[catch {

set server [socket -server ::1 0]

set port [lindex [chan configure $server -sockname] 5]

if {[catch {

set server [socket -server dummy -myaddr ::1 0]

set port [lindex [chan configure $server -sockname] 2]

zuiderkwast · 2025-10-09T22:40:22Z

tests/test_helper.tcl

+#Check if is required the flag to decide if run the tests or not
+if {!$::require_ipv6 && ![is_ipv6_available]} {
+    lappend ::denytags "ipv6"
+    puts "IPv6 not available on this system, skipping IPv6 tests"
+}


I can see this will work, but comparing with other similar mechanisms to skip tests in various conditions, this logic should better be placed in the function tags_acceptable in the file tests/support/server.tcl. Add it next to these checks, here: https://github.com/valkey-io/valkey/blob/9.0.0-rc3/tests/support/server.tcl#L266

…check - Fixed minor typos across test files. - Updated to include the new option: - Moved IPv6 availability check to tests/support/server.tcl. Signed-off-by: diego-ciciani01 <[email protected]>

github-actions bot assigned diego-ciciani01 Oct 2, 2025

zuiderkwast reviewed Oct 6, 2025

View reviewed changes

zuiderkwast mentioned this pull request Oct 8, 2025

Add compatibility test with Valkey 7.2/8.0 #2342

Merged

hpatro mentioned this pull request Oct 8, 2025

[NEW] Add mechanism to mark a test as skipped within the test logic dynamically #2711

Open

diego-ciciani01 added 3 commits October 9, 2025 07:30

diego-ciciani01 force-pushed the fix-ipv6-test-detection branch from 5de3e86 to 63e36cb Compare October 9, 2025 15:32

zuiderkwast reviewed Oct 9, 2025

View reviewed changes

diego-ciciani01 added 2 commits October 10, 2025 15:50

Merge branch 'valkey-io:unstable' into fix-ipv6-test-detection

d77f90d

fix: correct typos, update help text, and relocate IPv6 availability …

8c0c442

…check - Fixed minor typos across test files. - Updated to include the new option: - Moved IPv6 availability check to tests/support/server.tcl. Signed-off-by: diego-ciciani01 <[email protected]>

		#IPv6 detection utilities
		#for this detaction: the socket connection on ::1 address

Add IPv6 availability check to skip tests when unavailable #2674

Are you sure you want to change the base?

Add IPv6 availability check to skip tests when unavailable #2674

Conversation

diego-ciciani01 commented Oct 2, 2025

Problem e test suite fails when IPv6 is not available on the system with the error:

Solution

Testing

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuiderkwast Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuiderkwast Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

diego-ciciani01 commented Oct 7, 2025

Uh oh!

codecov bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zuiderkwast Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zuiderkwast Oct 7, 2025 •

edited

Loading

zuiderkwast Oct 9, 2025 •

edited

Loading

codecov bot commented Oct 7, 2025 •

edited

Loading

zuiderkwast Oct 9, 2025 •

edited

Loading