issue: HPCINFRA-3949 refine remote address configuration

[tomerdbz]

Description

Simplify test configuration by using supplied addresses from --addr flag
for all connection tests, eliminating gateway detection complexity.

Key improvements:

1. Use supplied addresses for connection tests

   • tcp_event.ti_2: Connect to localhost (127.0.0.1/::1) port 8889
   • udp_connect.ti_2: Connect to server_addr from --addr flag
   • Simpler configuration: only one remote address parameter (-r) needed

2. Fix bogus_addr port handling

   • Set bogus_addr port to 8888 (was 0, which is invalid for connect)
   • Fixes udp_connect.ti_3 failure (errno 9 - EBADF)

3. Improve semantic correctness

   • udp_connect.ti_3: Use remote_unreachable_addr instead of bogus_addr
     to match test description ("connect to unreachable ip")

Changes:

• tcp_event.ti_2: Use localhost with port 8889 for connection refusal test
• udp_connect.ti_2: Use server_addr instead of gateway address
• udp_connect.ti_3: Use remote_unreachable_addr instead of bogus_addr
• Fix bogus_addr port initialization: 0 → 8888
• Simplify main.cc configuration logic

What

gtest: simplify remote addresses handling.

Why ?

support gtest container migration, as current spawned containers don't have gateways.

How ?

It is optional but for complex PRs please provide information about the design,
architecture, approach, etc.

Change type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Tests
• Other

Check list

• Code follows the style de facto guidelines of this project
• Comments have been inserted in hard to understand places
• Documentation has been updated (if necessary)
• Test has been added (if possible)

[greptile-apps]

Greptile Overview

Greptile Summary

This PR improves test portability by distinguishing between routable and non-routable remote addresses, enabling tests to run in containerized environments without gateways.

Key improvements:

• Split remote_addr into remote_unreachable_addr (for bind failure tests) and remote_routable_addr (for connect tests)
• Added RFC 5737 TEST-NET-1 (192.0.2.1) as default IPv4 unreachable address instead of 0.0.0.0
• Implemented link-local filtering (sys_is_link_local()) to detect invalid gateways (169.254.x.x, fe80::)
• Gateway detection now falls back to localhost when no valid gateway is found
• tcp_event.ti_2 gracefully skips when running with localhost via GTEST_SKIP()
• Added -g/--remote-routable flag for users to specify routable address explicitly
• Consolidated gateway detection logic into set_def_remote_address()
• Extracted convert_and_copy_address() helper to reduce code duplication

Impact:

• Tests now work in containerized environments without external gateways
• Better semantic clarity between addresses used for different test purposes
• Improved user experience with clearer command-line options

Confidence Score: 4/5

• PR is safe to merge with minor review of gateway detection edge cases
• The refactoring is well-structured and improves code clarity. The link-local detection is correctly implemented according to RFC 3927 (IPv4) and RFC 4291 (IPv6). The main concern is the gateway detection logic that might populate remote_routable_addr with a gateway address before checking if it's link-local - this could leave the structure in an unexpected state if sys_gateway() succeeds but finds a link-local address. However, the localhost fallback handles this gracefully
• tests/gtest/main.cc - verify gateway detection logic handles all edge cases correctly

Important Files Changed

File Analysis

Filename	Score	Overview
tests/gtest/common/base.cc	5/5	Extracted convert_and_copy_address() helper to eliminate duplication; added conversion for new remote_routable_addr and renamed remote_addr to remote_unreachable_addr
tests/gtest/common/sys.cc	5/5	Added sys_is_link_local() to detect link-local addresses (169.254.x.x for IPv4, fe80::/10 for IPv6)
tests/gtest/main.cc	4/5	Consolidated gateway detection in set_def_remote_address(); added -g flag for routable address; changed defaults to use RFC 5737 192.0.2.1 for unreachable IPv4 and localhost for routable
tests/gtest/tcp/tcp_event.cc	5/5	Replaced def_gw_exists check with localhost detection and GTEST_SKIP; uses remote_routable_addr for gateway-based tests

Sequence Diagram

sequenceDiagram
    participant User
    participant main.cc
    participant set_def_remote_address
    participant sys_gateway
    participant sys_is_link_local
    participant test_base
    participant test

    User->>main.cc: Launch with optional -g/-r flags
    main.cc->>main.cc: _def_config() sets defaults<br/>(192.0.2.1 unreachable, 127.0.0.1 routable)
    main.cc->>main.cc: Parse command line args<br/>Set user_defined_routable, user_defined_unreachable
    main.cc->>set_def_remote_address: Call with flags
    
    alt user_defined_unreachable is false AND IPv6
        set_def_remote_address->>set_def_remote_address: Set unreachable to fdff:ffff:...:ffff
    else
        set_def_remote_address->>set_def_remote_address: Just set port to 8888
    end
    
    alt user_defined_routable is false
        set_def_remote_address->>sys_gateway: Detect gateway for family
        sys_gateway-->>set_def_remote_address: Gateway address (or not found)
        set_def_remote_address->>sys_is_link_local: Check if gateway is link-local
        sys_is_link_local-->>set_def_remote_address: true/false
        
        alt Gateway found AND not link-local
            set_def_remote_address->>set_def_remote_address: Use gateway, set port 8888
        else No valid gateway
            set_def_remote_address->>set_def_remote_address: Fallback to localhost
        end
    else user provided -g
        set_def_remote_address->>set_def_remote_address: Just set port to 8888
    end
    
    main.cc->>test_base: Constructor called
    test_base->>test_base: convert_and_copy_address()<br/>for all addresses
    test_base-->>main.cc: Ready
    
    main.cc->>test: Run tests
    
    alt tcp_event.ti_2 with localhost
        test->>test: Check if remote_routable_addr is localhost
        test->>test: GTEST_SKIP()
    else tcp_event.ti_2 with real gateway
        test->>test: Connect to remote_routable_addr
        test->>test: Assert EINPROGRESS and events
    end
    
    alt bind tests (tcp_bind.ti_2, udp_bind.ti_2)
        test->>test: Bind to remote_unreachable_addr
        test->>test: Assert EADDRNOTAVAIL
    end

Loading

_{12 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR refactors test infrastructure to improve remote address handling and eliminate gateway dependencies for container environments.

Key Changes:

• Renamed remote_addr → remote_unreachable_addr for semantic clarity across all test files
• Adopted RFC 5737 TEST-NET-1 (192.0.2.1) for IPv4 and RFC 3849 (2001:db8::1) for IPv6 as unreachable defaults
• Removed sys_gateway() function and def_gw_exists flag that previously relied on route command
• Extracted convert_and_copy_address() helper in base.cc to eliminate code duplication
• Updated tests: tcp_event.ti_2 now uses localhost instead of gateway; bind tests use unreachable addresses
• Removed SKIP_TRUE(def_gw_exists) checks from tcp_bind.ti_2, udp_bind.ti_2, and connect tests

Issues Found:

• Address family conversion assumes source is IPv4 when families don't match, lacks validation
• No IPv6→IPv4 conversion path if user provides IPv6 but system defaults to IPv4
• Hardcoded port 8889 in tcp_event.ti_2 may cause conflicts
• Unrelated hash.c file included in PR

Confidence Score: 3/5

• This PR has logical issues with address family conversion that could cause runtime failures in mixed IPv4/IPv6 scenarios
• Score reflects critical logic gaps in convert_and_copy_address() that assumes non-matching families are always IPv4→IPv6, plus hardcoded port conflicts. The refactoring improves test portability but introduces conversion bugs that need fixing before merge.
• Pay close attention to tests/gtest/common/base.cc (address conversion logic) and tests/gtest/main.cc (configuration setup)

Important Files Changed

File Analysis

Filename	Score	Overview
tests/gtest/main.cc	2/5	Refactored address configuration with RFC 5737/3849 defaults; removed gateway detection logic; IPv4→IPv6 conversion path missing
tests/gtest/common/base.cc	2/5	Extracted convert_and_copy_address helper; lacks address family validation for non-IPv4 sources
tests/gtest/tcp/tcp_event.cc	3/5	Changed test to use localhost instead of gateway; hardcoded port 8889 may conflict
tests/gtest/hash.c	3/5	New file including ../../tools/daemon/hash.c; appears unrelated to address configuration changes

Sequence Diagram

sequenceDiagram
    participant User
    participant main
    participant set_def_remote_address
    participant test_base
    participant convert_and_copy_address
    participant Tests

    User->>main: Start tests with --addr flag
    main->>main: _def_config()
    Note over main: Set IPv4 default: 192.0.2.1[8888]
    main->>main: _set_config(argc, argv)
    
    alt User provides --addr
        main->>main: Parse --addr, set client/server_addr
    end
    
    alt User provides --remote-non-routable
        main->>main: Parse -r, set remote_unreachable_addr
        Note over main: user_defined_unreachable = true
    end
    
    main->>set_def_remote_address: Call with user_defined_unreachable
    
    alt IPv6 family && !user_defined_unreachable
        set_def_remote_address->>set_def_remote_address: Use 2001:db8::1[8888]
    else IPv6 family && user_defined_unreachable
        set_def_remote_address->>set_def_remote_address: Set port to 8888
    else IPv4 family
        set_def_remote_address->>set_def_remote_address: Set port to 8888
    end
    
    main->>Tests: RUN_ALL_TESTS()
    Tests->>test_base: Constructor
    test_base->>test_base: Determine m_family from server/client addr
    
    test_base->>convert_and_copy_address: Convert client_addr to m_family
    alt source family != target family
        convert_and_copy_address->>convert_and_copy_address: sys_ipv4_to_ipv6()
        Note over convert_and_copy_address: Assumes source is IPv4!
    else source family == target family
        convert_and_copy_address->>convert_and_copy_address: memcpy
    end
    
    test_base->>convert_and_copy_address: Convert server_addr to m_family
    test_base->>convert_and_copy_address: Convert remote_unreachable_addr to m_family
    
    Tests->>Tests: Execute test cases
    Note over Tests: Use remote_unreachable_addr for bind failures<br/>Use localhost for connection tests

Loading

Additional Comments (1)

1. tests/gtest/main.cc, line 16 (link)

   logic: IPv4-to-IPv6 conversion only (no reverse). convert_and_copy_address() in base.cc:16 only handles IPv4→IPv6 via sys_ipv4_to_ipv6(), with no IPv6→IPv4 conversion path. If user provides IPv6 address via --addr but system defaults to IPv4, conversion will fail silently or cause runtime issues.

_{12 files reviewed, 5 comments}

_{Edit Code Review Agent Settings | Greptile}

[tomerdbz]

Validated PR makes gtest suite run on container target w/o any failures.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR simplifies test configuration by removing gateway detection complexity and using user-supplied or default unreachable addresses for connection tests.

Key Changes:

• Removed sys_gateway() function that parsed routing tables
• Renamed remote_addr to remote_unreachable_addr for semantic clarity
• Replaced gateway-based address detection with explicit unreachable addresses (192.0.2.1 for IPv4, 2001:db8::1 for IPv6)
• Updated tests to use appropriate addresses: tcp_event.ti_2 uses localhost:8889, udp_connect.ti_2 uses server_addr, connection refusal tests use remote_unreachable_addr
• Refactored address conversion logic into reusable helper function
• Removed def_gw_exists flag and associated gateway dependency checks

Issues Found:

• PR description claims bogus_addr port was fixed (0→8888), but the code still sets it to 0. The actual fix was replacing bogus_addr usage with remote_unreachable_addr in tests.
• convert_and_copy_address() validation could be clearer about assertion order (though current implementation is safe)

Confidence Score: 4/5

• This PR is safe to merge with minimal risk - changes are well-contained to test infrastructure
• The refactoring successfully simplifies test configuration by removing gateway detection complexity. The changes are mostly mechanical renamings and use clearer, more explicit address handling. However, there's a minor discrepancy between the PR description and actual implementation regarding bogus_addr, and the hardcoded port 8889 in tcp_event.ti_2 could cause conflicts in some environments (as noted in previous comments).
• Pay attention to tests/gtest/tcp/tcp_event.cc (hardcoded port could conflict) and tests/gtest/common/base.cc (misleading PR description about bogus_addr port fix)

Important Files Changed

File Analysis

Filename	Score	Overview
tests/gtest/common/base.cc	4/5	Refactored address conversion logic into helper function; changed from remote_addr to remote_unreachable_addr
tests/gtest/main.cc	4/5	Simplified remote address configuration; replaced gateway detection with user-supplied or default unreachable addresses
tests/gtest/tcp/tcp_event.cc	3/5	Changed ti_2 to use hardcoded localhost:8889; ti_4 changed to use server_addr instead of remote_addr
tests/gtest/udp/udp_connect.cc	4/5	ti_2 now uses server_addr; ti_3 changed from bogus_addr to remote_unreachable_addr and added IPv6 skip

Sequence Diagram

sequenceDiagram
    participant User
    participant main.cc
    participant test_base
    participant Tests
    
    User->>main.cc: Start gtest with --addr flag
    main.cc->>main.cc: _def_config() - Set defaults
    Note over main.cc: client_addr = 0.0.0.0[0]<br/>server_addr = 0.0.0.0[0]<br/>remote_unreachable_addr = 192.0.2.1[8888]
    
    User->>main.cc: --addr <client_ip>,<server_ip>
    main.cc->>main.cc: Parse --addr flag
    main.cc->>main.cc: set_def_remote_address()
    
    alt IPv6 && no user-defined unreachable
        main.cc->>main.cc: Set remote_unreachable_addr = 2001:db8::1[8888]
    else User provided unreachable addr without port
        main.cc->>main.cc: Set port to 8888
    end
    
    main.cc->>test_base: Initialize test (constructor)
    test_base->>test_base: convert_and_copy_address(client_addr)
    test_base->>test_base: convert_and_copy_address(server_addr)
    test_base->>test_base: convert_and_copy_address(remote_unreachable_addr)
    
    alt source_family != target_family
        test_base->>test_base: Validate AF_INET → AF_INET6
        test_base->>test_base: sys_ipv4_to_ipv6()
    else Same family
        test_base->>test_base: memcpy addresses
    end
    
    test_base->>Tests: Ready for testing
    
    Tests->>Tests: tcp_event.ti_2: connect to localhost:8889
    Tests->>Tests: udp_connect.ti_2: connect to server_addr
    Tests->>Tests: udp_connect.ti_3: connect to remote_unreachable_addr
    Tests->>Tests: tcp/udp_bind.ti_2: bind to remote_unreachable_addr

Loading

Additional Comments (1)

1. tests/gtest/common/base.cc, line 52 (link)

   style: PR description claims bogus_addr port was changed from 0 to 8888, but code still sets port to 0. While bogus_addr is no longer used in the modified tests (replaced with remote_unreachable_addr), this creates confusion and leaves dead code with an invalid configuration that could cause issues if bogus_addr is used elsewhere.

_{11 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Greptile Overview

Greptile Summary

Simplifies test configuration by eliminating gateway detection complexity and using supplied addresses from --addr flag for connection tests.

Key changes:

• Removed sys_gateway() function and gateway detection logic from test infrastructure
• Renamed remote_addr to remote_unreachable_addr for semantic clarity
• Updated tests to use server_addr (from --addr flag) instead of detected gateway addresses
• Changed tcp_event.ti_2 to connect to localhost:8889 for connection refusal test
• Refactored address conversion logic into convert_and_copy_address() helper function
• Removed def_gw_exists flag throughout codebase

Issues found:

• Critical: bogus_addr port still set to 0 in base.cc:52, contradicting PR description claim that it's fixed to 8888

Confidence Score: 3/5

• This PR has a critical bug where bogus_addr port remains 0 instead of 8888 as claimed.
• Score reflects incomplete implementation - the PR description claims bogus_addr port was fixed from 0 to 8888 to resolve udp_connect.ti_3 failures, but the actual code still sets port to 0. This contradicts the stated goal and may not fix the test failure. The refactoring itself is sound, but the missing fix reduces confidence.
• tests/gtest/common/base.cc requires attention - bogus_addr port must be set to 8888

Important Files Changed

File Analysis

Filename	Score	Overview
tests/gtest/common/base.cc	3/5	Refactored address conversion logic into convert_and_copy_address() helper. However, bogus_addr port still set to 0 (not 8888 as claimed in PR description).
tests/gtest/main.cc	4/5	Simplified address configuration by removing gateway detection, using default unreachable addresses (192.0.2.1 for IPv4, 2001:db8::1 for IPv6). Renamed flag to --remote-non-routable.
tests/gtest/tcp/tcp_event.cc	4/5	Changed ti_2 test to connect to localhost (127.0.0.1/::1) port 8889 instead of remote address. Removed def_gw_exists check. Hardcoded port may conflict.
tests/gtest/udp/udp_connect.cc	4/5	ti_2 now uses server_addr (routable) instead of gateway. ti_3 uses remote_unreachable_addr instead of bogus_addr (semantic fix). Removed def_gw_exists checks.

Sequence Diagram

sequenceDiagram
    participant User
    participant main.cc
    participant test_base
    participant Tests
    
    User->>main.cc: Run gtest with --addr flag
    main.cc->>main.cc: _def_config() - Set defaults<br/>(192.0.2.1[8888] unreachable)
    main.cc->>main.cc: _set_config() - Parse --addr
    main.cc->>main.cc: set_def_remote_address()<br/>(Use 2001:db8::1 for IPv6)
    
    Note over main.cc: No gateway detection<br/>Uses supplied addresses
    
    main.cc->>test_base: Initialize test suite
    test_base->>test_base: convert_and_copy_address()<br/>(client, server, unreachable)
    test_base->>test_base: Initialize bogus_addr<br/>(port=0, IP=1.1.1.1)
    
    test_base->>Tests: Run test cases
    
    alt tcp_event.ti_2
        Tests->>Tests: Connect to localhost:8889<br/>(Expects ECONNREFUSED)
    else udp_connect.ti_2
        Tests->>Tests: Connect to server_addr<br/>(Uses --addr value)
    else udp_connect.ti_3
        Tests->>Tests: Connect to remote_unreachable_addr<br/>(Expects no error for UDP)
    else bind tests (ti_2)
        Tests->>Tests: Bind to remote_unreachable_addr<br/>(Expects EADDRNOTAVAIL)
    end

Loading

Additional Comments (1)

1. tests/gtest/common/base.cc, line 52 (link)

   logic: Port still set to 0, not 8888 as claimed in PR description. If the goal was to fix udp_connect.ti_3 by setting port to 8888, this change is missing.

_{11 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Greptile Overview

Greptile Summary

This PR successfully simplifies the test suite's remote address configuration by eliminating complex gateway detection logic. The changes support containerized test environments that lack default gateways.

Key Changes:

• Removed sys_gateway() function and gateway detection via shell commands (route -n)
• Renamed remote_addr → remote_unreachable_addr for semantic clarity
• Removed def_gw_exists flag and associated SKIP_TRUE checks across tests
• Refactored address conversion logic into reusable convert_and_copy_address() helper
• Updated CLI option from --remote to --remote-unreachable with clearer semantics
• Tests now use simpler, more predictable addresses:
  • tcp_event.ti_2: localhost:8889 (connection refusal)
  • udp_connect.ti_2: server_addr from --addr flag (routable)
  • udp_connect.ti_3: remote_unreachable_addr (unreachable, defaults to 192.0.2.1:8888 or 2001:db8::1:8888)

Code Quality:
The refactoring improves maintainability by replacing environment-dependent shell commands with deterministic address configurations. The new convert_and_copy_address() helper eliminates duplicate conversion code.

Testing Note:
IPv6 test udp_connect.ti_3 now skips on bare-metal due to routing limitations, awaiting containerized environment migration.

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The refactoring eliminates environment-dependent gateway detection in favor of explicit, deterministic address configuration. All changes are test-only with no impact on production code. The code simplification reduces complexity while maintaining test coverage. Previous review comments about hardcoded port 8889 are noted but don't block merge as tests handle port conflicts appropriately.
• No files require special attention. The refactoring is well-structured and improves code quality across all modified files.

Important Files Changed

File Analysis

Filename	Score	Overview
tests/gtest/common/base.cc	5/5	Refactored address conversion logic into helper function convert_and_copy_address(), removed def_gw_exists field, and changed remote_addr to remote_unreachable_addr
tests/gtest/main.cc	4/5	Simplified address configuration by removing gateway detection, updated CLI option from --remote to --remote-unreachable, and improved IPv6 unreachable address defaults
tests/gtest/tcp/tcp_event.cc	5/5	Updated to use localhost (127.0.0.1/::1) with port 8889 for connection refusal test, removed gateway check
tests/gtest/udp/udp_connect.cc	5/5	Changed ti_2 to use server_addr (routable), ti_3 to use remote_unreachable_addr (unreachable), removed gateway checks, added IPv6 skip for bare-metal

Sequence Diagram

sequenceDiagram
    participant User
    participant main.cc
    participant gtest_conf
    participant test_base
    participant Tests

    User->>main.cc: Start with CLI args (--addr, --remote-unreachable)
    main.cc->>main.cc: _def_config()
    Note over main.cc: Set defaults:<br/>client: 0.0.0.0[0]<br/>server: 0.0.0.0[0]<br/>unreachable: 192.0.2.1[8888]
    
    main.cc->>main.cc: _set_config()
    alt User provided --addr
        main.cc->>gtest_conf: Parse client,server IPs
    end
    
    alt User provided --remote-unreachable
        main.cc->>gtest_conf: Set remote_unreachable_addr
    end
    
    main.cc->>main.cc: set_def_remote_address()
    alt IPv6 && no user unreachable addr
        main.cc->>gtest_conf: Set to 2001:db8::1[8888]
    else User provided addr without port
        main.cc->>gtest_conf: Default port to 8888
    end
    
    main.cc->>Tests: RUN_ALL_TESTS()
    Tests->>test_base: Constructor
    test_base->>test_base: Determine m_family from gtest_conf
    test_base->>test_base: convert_and_copy_address(client_addr)
    test_base->>test_base: convert_and_copy_address(server_addr)
    test_base->>test_base: convert_and_copy_address(remote_unreachable_addr)
    alt IPv4 to IPv6 conversion needed
        test_base->>test_base: sys_ipv4_to_ipv6()
    end
    test_base->>test_base: Initialize bogus_addr (1.1.1.1:0)
    
    test_base->>Tests: Ready with addresses
    Tests->>Tests: Execute test cases<br/>(tcp_event.ti_2: localhost:8889)<br/>(udp_connect.ti_2: server_addr)<br/>(udp_connect.ti_3: remote_unreachable_addr)

Loading

_{11 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[tomerdbz]

bot:retest

[greptile-apps]

Greptile Overview

Greptile Summary

This PR simplifies the gtest configuration by eliminating gateway detection logic and replacing it with explicit unreachable addresses. The changes enable tests to run in containerized environments without gateway access.

Key Changes:

• Removes sys_gateway() function and def_gw_exists flag that relied on executing route commands
• Renames remote_addr to remote_unreachable_addr for semantic clarity
• Uses RFC-documented unreachable addresses: 192.0.2.1 (IPv4) and 2001:db8::1 (IPv6)
• Updates test cases to use server_addr for routable connections and remote_unreachable_addr for unreachable scenarios
• tcp_event.ti_2 now connects to localhost:8889 for connection refusal testing
• Refactors address conversion into reusable convert_and_copy_address() helper

Issues Found:

• Critical: bogus_addr port remains 0 despite PR description claiming fix to 8888 (tests/gtest/common/base.cc:52)
• The PR description mentions fixing bogus_addr but the code doesn't implement this change

Confidence Score: 3/5

• This PR improves test portability but has a critical discrepancy between description and implementation
• The refactoring is sound and achieves the stated goal of removing gateway dependencies, but the PR description claims to fix bogus_addr port from 0 to 8888 while the code still sets it to 0. This contradiction raises concerns about whether testing was performed properly or if requirements were misunderstood. The actual functional impact may be limited since tests now use remote_unreachable_addr instead of bogus_addr
• tests/gtest/common/base.cc - verify bogus_addr port should be 0 or 8888

Important Files Changed

File Analysis

Filename	Score	Overview
tests/gtest/common/base.cc	3/5	Refactors address conversion into helper function, updates to use remote_unreachable_addr, but bogus_addr port still 0 despite PR claiming fix to 8888
tests/gtest/main.cc	4/5	Replaces gateway detection with explicit unreachable addresses; defaults to RFC 3849 (2001:db8::1) for IPv6 and RFC 5737 (192.0.2.1) for IPv4
tests/gtest/tcp/tcp_event.cc	4/5	ti_2 now connects to localhost:8889 instead of remote_addr; ti_4 uses server_addr; removes def_gw_exists check
tests/gtest/udp/udp_connect.cc	4/5	ti_2 uses server_addr (routable), ti_3 uses remote_unreachable_addr instead of bogus_addr; adds IPv6 skip for ti_3; removes def_gw_exists checks

Sequence Diagram

sequenceDiagram
    participant User
    participant main.cc
    participant test_base
    participant Test Cases
    
    User->>main.cc: Start gtest with --addr flag
    main.cc->>main.cc: _def_config() sets defaults
    Note over main.cc: IPv4: 192.0.2.1:8888<br/>IPv6: will be set later
    
    main.cc->>main.cc: _set_config() parses args
    alt User provides --remote-non-routable
        main.cc->>main.cc: Parse custom unreachable addr
        main.cc->>main.cc: user_defined_unreachable = true
    end
    
    main.cc->>main.cc: set_def_remote_address()
    alt IPv6 && !user_defined
        main.cc->>main.cc: Set 2001:db8::1:8888
    end
    alt user_defined && port == 0
        main.cc->>main.cc: Set port to 8888
    end
    
    Test Cases->>test_base: Construct test_base()
    test_base->>test_base: convert_and_copy_address()
    Note over test_base: Converts IPv4→IPv6 if needed
    test_base->>test_base: Set bogus_addr (port=0)
    
    alt tcp_event.ti_2
        Test Cases->>Test Cases: Connect to localhost:8889
        Note over Test Cases: Expect ECONNREFUSED
    end
    
    alt udp_connect.ti_2
        Test Cases->>Test Cases: Connect to server_addr
        Note over Test Cases: Uses --addr value
    end
    
    alt udp_connect.ti_3
        Test Cases->>Test Cases: Connect to remote_unreachable_addr
        Note over Test Cases: Uses RFC unreachable address
    end

Loading

Additional Comments (1)

1. tests/gtest/common/base.cc, line 52 (link)

   logic: Port still set to 0, contradicting PR description. The PR description states "Set bogus_addr port to 8888 (was 0, which is invalid for connect)" but port remains 0. If bogus_addr was meant to be fixed for udp_connect.ti_3 (which now uses remote_unreachable_addr), then this might be okay. However, the PR description is misleading and needs clarification.

_{11 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}