Skip to content

Add TCP and UDP network unit tests#162

Open
222448082Ashen wants to merge 1 commit intothoth-tech:mainfrom
222448082Ashen:unit/test/networking
Open

Add TCP and UDP network unit tests#162
222448082Ashen wants to merge 1 commit intothoth-tech:mainfrom
222448082Ashen:unit/test/networking

Conversation

@222448082Ashen
Copy link
Copy Markdown

Description

This PR expands networking unit test coverage by porting core TCP and UDP message-flow checks from sktest into the Catch2 unit test suite.

Changes included:

  • Added TCP message round-trip test in unit_test_network.cpp
  • Added UDP message round-trip test in unit_test_network.cpp
  • Added a small polling helper for async network activity to reduce timing flakiness in unit tests
  • Added missing functional header/namespace usage required by the helper in unit_test_network.cpp

Motivation:

  • Existing sktest had TCP/UDP message tests, but equivalent coverage was missing in skunit_tests.
  • This brings automated unit coverage closer to behavior already manually verified in sktest.

Dependencies:

  • No new project dependencies added.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (update or new)

How Has This Been Tested?

Build and discovery:

  1. Configure CMake Ninja build for tests
  2. Build skunit_tests target
  3. Verify test discovery with ctest -N

Networking test execution:

  1. Run filtered networking tests through CTest:
    ctest --test-dir projects/cmake/build-ninja -R "network|server|communicate|tcp messages|udp messages" --output-on-failure
  2. Confirmed new test cases are discovered:
    • tcp messages can flow both ways
    • udp messages can flow both ways

Current environment result:

  • Test execution in this Windows environment fails at process startup with exit code 0xc0000139 across networking cases (runtime DLL/entrypoint issue), including pre-existing networking tests.
  • This indicates an environment/runtime problem rather than an assertion failure specific to the newly added tests.

Testing Checklist

  • Tested with sktest
  • Tested with skunit_tests

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have requested a review from ... on the Pull Request

Extend network unit tests to improve coverage and handle asynchronous network activity. Added a wait_for_network_condition helper (uses current_ticks, check_network_activity, delay) and includes (<functional>, utils.h). Added two new tests in unit_test_network.cpp: "tcp messages can flow both ways" (port 3010) and "udp messages can flow both ways" (port 3011) which verify bidirectional messaging between server and client, message contents, and proper cleanup of connections/servers. Also added CTest temporary output files (CTestCostData.txt, LastTest.log, LastTestsFailed.log) produced by the test run.
Copy link
Copy Markdown

@kyriesk kyriesk left a comment

Choose a reason for hiding this comment

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

Positive Aspects

  • The test is well-structured and covers the full TCP message flow in both directions, which is essential for robust networking.
  • The test cleans up resources (servers, connections) at the end, which helps maintain test isolation and reliability.

Fail Test Identified

Line 275

FAILED:
  REQUIRE( accept_new_connection(server) )
with expansion:
  false

Issue: This assertion is failing, indicating that the server did not accept a new connection from the client. This can be due to timing issues, where the client connects before the server is fully ready, or the server does not detect the new connection in time.
Suggestion: Consider wrapping accept_new_connection(server) in a retry loop or use wait_for_network_condition to repeatedly check until the connection is accepted or a timeout occurs. This will make the test more robust against timing-related flakiness.

Additional Suggestions:

Add more detailed logging or error messages when the connection is not accepted, to help diagnose whether the issue is on the client or server side.
If possible, verify that the client is actually attempting to connect and that the server is listening on the expected port before making assertions.

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