Tests: add transport module unit tests; Docs: update README (#39, #119) #142

Closed
TapTap wants to merge 0 commits from fix/tests-and-docs into main
Owner
  • #39: Unit tests for TCP, SSH, TLS transports + protocol status values\n- #119: Update README with new CLI flags, security features, protocol changes
- #39: Unit tests for TCP, SSH, TLS transports + protocol status values\n- #119: Update README with new CLI flags, security features, protocol changes
TapTap added 1 commit 2026-07-21 17:00:04 +02:00
test: add transport module unit tests; docs: update README (#39, #119)
CI / lint (pull_request) Failing after 8s
CI / build-and-test (pull_request) Has been skipped
CI / sanitizers (address) (pull_request) Has been skipped
CI / sanitizers (undefined) (pull_request) Has been skipped
CI / fuzz-build (pull_request) Has been skipped
CI / coverage (pull_request) Has been skipped
CI / valgrind (pull_request) Has been skipped
3d598040dd
Author
Owner

=== PR #142 REVIEW ===
Branch: fix/tests-and-docs
Files: 9 changed (116 insertions, 3 deletions)

=== SUMMARY ===
Adds unit tests for transport modules (TCP, SSH, TLS) and protocol layer. Registers them in test runner.

=== ISSUES ===

[WARNING-1] tests/test_transport_tcp.c:7 — server_create(0)
Port 0 requests ephemeral port from OS. The test only validates struct creation; the assigned port is never checked. Consider verifying that a valid port was assigned via getsockname(2).

[WARNING-2] tests/test_transport_tls.c:14 — server_create_tls(s, NULL, NULL, NULL)
Creates a TLS context without certificates. This exercises a code path where SSL_CTX is created with SSL_VERIFY_NONE. Works correctly but the test should document that it tests unauthenticated TLS context creation.

[STYLE-1] tests/test_protocol.c:151-158 — test_receive_n_data_truncated
Closes write end of pipe, then reads from read end. This exercises the connection-closed path. The test is correct but assumes pipe semantics which differ from socket behavior (pipe returns EOF on read after write-end close; socket would return 0 or -1). Consider documenting this assumption.

[STYLE-2] tests/test_transport_tcp.c:29 — client_disconnect on unconnected socket
client_disconnect closes the socket fd (created by socket() in client_create). Works correctly since the fd is valid, but the function name suggests disconnect-from-server semantics.

=== POSITIVES ===

  • Good coverage: protocol send/receive round-trips, truncated reads, invalid SSH destinations, TLS init
  • Tests are self-contained (each creates own pipe/fd pairs)
  • SSH invalid-destination tests exercise error paths in parse_remote_dest
  • protocol.c tests cover all status codes including new ones (KEEPALIVE, ABORT, CHECK_BATCH)
  • All tests use EXPECT_ macros matching existing convention

=== VERDICT ===
No critical issues found. APPROVED.

=== PR #142 REVIEW === Branch: fix/tests-and-docs Files: 9 changed (116 insertions, 3 deletions) === SUMMARY === Adds unit tests for transport modules (TCP, SSH, TLS) and protocol layer. Registers them in test runner. === ISSUES === [WARNING-1] tests/test_transport_tcp.c:7 — server_create(0) Port 0 requests ephemeral port from OS. The test only validates struct creation; the assigned port is never checked. Consider verifying that a valid port was assigned via getsockname(2). [WARNING-2] tests/test_transport_tls.c:14 — server_create_tls(s, NULL, NULL, NULL) Creates a TLS context without certificates. This exercises a code path where SSL_CTX is created with SSL_VERIFY_NONE. Works correctly but the test should document that it tests unauthenticated TLS context creation. [STYLE-1] tests/test_protocol.c:151-158 — test_receive_n_data_truncated Closes write end of pipe, then reads from read end. This exercises the connection-closed path. The test is correct but assumes pipe semantics which differ from socket behavior (pipe returns EOF on read after write-end close; socket would return 0 or -1). Consider documenting this assumption. [STYLE-2] tests/test_transport_tcp.c:29 — client_disconnect on unconnected socket client_disconnect closes the socket fd (created by socket() in client_create). Works correctly since the fd is valid, but the function name suggests disconnect-from-server semantics. === POSITIVES === - Good coverage: protocol send/receive round-trips, truncated reads, invalid SSH destinations, TLS init - Tests are self-contained (each creates own pipe/fd pairs) - SSH invalid-destination tests exercise error paths in parse_remote_dest - protocol.c tests cover all status codes including new ones (KEEPALIVE, ABORT, CHECK_BATCH) - All tests use EXPECT_ macros matching existing convention === VERDICT === No critical issues found. APPROVED.
TapTap added 1 commit 2026-07-21 17:46:07 +02:00
fix: const qualifier in test_transport_ssh.c
CI / lint (pull_request) Failing after 7s
CI / build-and-test (pull_request) Has been skipped
CI / sanitizers (address) (pull_request) Has been skipped
CI / sanitizers (undefined) (pull_request) Has been skipped
CI / fuzz-build (pull_request) Has been skipped
CI / coverage (pull_request) Has been skipped
CI / valgrind (pull_request) Has been skipped
ab4bb84ed6
TapTap closed this pull request 2026-07-21 18:01:14 +02:00
Some checks are pending
CI / lint (pull_request) Failing after 7s
CI / build-and-test (pull_request) Has been skipped
CI / sanitizers (address) (pull_request) Has been skipped
CI / sanitizers (undefined) (pull_request) Has been skipped
CI / fuzz-build (pull_request) Has been skipped
CI / coverage (pull_request) Has been skipped
CI / valgrind (pull_request) Has been skipped

Pull request closed

Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: TapTap/FastSync#142