feat:[sal][utest] added test cases for the sal api#10720
feat:[sal][utest] added test cases for the sal api#10720Rbb666 merged 9 commits intoRT-Thread:masterfrom
Conversation
kurisaW
requested review from
BernardXiong,
Rbb666 and
supperthomas
as code owners
|
@Ryan-CW-Code 大佬帮忙review下 |
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive unit test coverage for the RT-Thread SAL (Socket Abstraction Layer) API. The tests validate socket operations including creation, binding, connection establishment, data transmission, and error handling scenarios.
- Adds a new test file
tc_sal_socket.cwith 1030 lines of socket API test coverage - Updates build configuration to include SAL test cases when enabled
- Reorganizes Kconfig structure to consolidate all network unit tests under a unified menu
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| components/net/utest/tc_sal_socket.c | Complete test suite for SAL socket operations with TCP/UDP communication tests |
| components/net/utest/SConscript | Adds SAL test source to build configuration |
| components/net/utest/Kconfig | Reorganizes network test configuration menu and adds SAL test option |
| int mode = 1; | ||
| sal_ioctlsocket(server_sock, FIONBIO, &mode); | ||
| LOG_D("Set server socket %d to non-blocking mode", server_sock); | ||
|
|
||
| int elapsed = 0; | ||
| int check_interval = 50; | ||
| int accepted_sock = -1; | ||
| while (elapsed < ACCEPT_POLL_TIMEOUT_MS) | ||
| { | ||
| accepted_sock = sal_accept(server_sock, RT_NULL, RT_NULL); | ||
| if (accepted_sock >= 0) | ||
| { | ||
| LOG_I("Accepted connection on socket %d", accepted_sock); | ||
| set_socket_timeout(accepted_sock, SOCKET_TIMEOUT_MS); | ||
| close_test_socket(accepted_sock); | ||
| break; | ||
| } |
There was a problem hiding this comment.
best_practices/最佳实践: The socket returned by sal_accept() should have its return value checked for errors before calling set_socket_timeout() and close_test_socket(). / sal_accept() 返回的套接字应该在调用 set_socket_timeout() 和 close_test_socket() 之前检查返回值是否有错误。
| sal_ioctlsocket(server_sock, FIONBIO, &mode); | ||
| LOG_D("Restored server socket %d to blocking mode", server_sock); |
There was a problem hiding this comment.
best_practices/最佳实践: The return value of sal_ioctlsocket() should be checked to ensure the socket mode was successfully restored to blocking. / 应该检查 sal_ioctlsocket() 的返回值以确保套接字模式成功恢复为阻塞模式。
| sal_ioctlsocket(server_sock, FIONBIO, &mode); | |
| LOG_D("Restored server socket %d to blocking mode", server_sock); | |
| int restore_ret = sal_ioctlsocket(server_sock, FIONBIO, &mode); | |
| if (restore_ret != 0) | |
| { | |
| LOG_E("Failed to restore server socket %d to blocking mode, ret=%d", server_sock, restore_ret); | |
| } | |
| else | |
| { | |
| LOG_D("Restored server socket %d to blocking mode", server_sock); | |
| } |
| int accepted_sock = sal_accept(server_sock, NULL, NULL); | ||
| if (accepted_sock < 0) | ||
| { | ||
| LOG_E("Server accept failed: %d", accepted_sock); | ||
| goto cleanup_sr; | ||
| } | ||
| LOG_I("Server accepted connection on socket %d", accepted_sock); |
There was a problem hiding this comment.
[nitpick] best_practices/最佳实践: Using NULL for address parameters in sal_accept() is acceptable for testing, but consider using proper sockaddr structures for more comprehensive testing coverage. / 在 sal_accept() 中使用 NULL 作为地址参数在测试中是可以接受的，但考虑使用适当的 sockaddr 结构以获得更全面的测试覆盖率。
| int accepted_sock = sal_accept(server_sock, NULL, NULL); | |
| if (accepted_sock < 0) | |
| { | |
| LOG_E("Server accept failed: %d", accepted_sock); | |
| goto cleanup_sr; | |
| } | |
| LOG_I("Server accepted connection on socket %d", accepted_sock); | |
| struct sockaddr_in peer_addr; | |
| socklen_t peer_addr_len = sizeof(peer_addr); | |
| int accepted_sock = sal_accept(server_sock, (struct sockaddr *)&peer_addr, &peer_addr_len); | |
| if (accepted_sock < 0) | |
| { | |
| LOG_E("Server accept failed: %d", accepted_sock); | |
| goto cleanup_sr; | |
| } | |
| char peer_ip[INET_ADDRSTRLEN]; | |
| inet_ntop(AF_INET, &peer_addr.sin_addr, peer_ip, sizeof(peer_ip)); | |
| LOG_I("Server accepted connection on socket %d from %s:%d", accepted_sock, peer_ip, ntohs(peer_addr.sin_port)); |
components/net/utest/SConscript
Outdated
| CPPPATH = [cwd] | ||
|
|
||
| if GetDepend('RT_UTEST_USING_ALL_CASES') or GetDepend('RT_UTEST_TC_USING_LWIP') or GetDepend('RT_UTEST_TC_USING_NETDEV'): | ||
| if GetDepend('RT_UTEST_USING_ALL_CASES'): |
There was a problem hiding this comment.
best_practices/最佳实践: The condition should also check for individual SAL test configuration to allow selective testing. Consider adding 'or GetDepend('RT_UTEST_TC_USING_SAL')' to the condition. / 条件应该也检查单独的 SAL 测试配置以允许选择性测试。考虑在条件中添加 'or GetDepend('RT_UTEST_TC_USING_SAL')'。
| if GetDepend('RT_UTEST_USING_ALL_CASES'): | |
| if GetDepend('RT_UTEST_USING_ALL_CASES') or GetDepend('RT_UTEST_TC_USING_SAL'): |
|
明天看看，这部分要不要跟 #10675 整合互补一下，抽一下公共代码？ |
|
sal testcase仅针对rtt sal组件提供api覆盖测试，本身sal就是对上支持标准bsd接口，向下对lwip，at组件等接口，这块的共性是肯定的，包括说后续的bsd socket，这块肯定也是有相似之处的。 个人建议保留对标准bsd socket，sal socket测试用例的覆盖 对于底层lwip，at等组件建议仅针对组件本身特色接口保留测试，其余socket通信接口由上层进行测试(最终会调用到底层lwip等接口) |
unicornx
left a comment
unicornx
left a comment
There was a problem hiding this comment.
只 review 了 utest 的框架使用，没有 review 具体的 tc_sal_socket.c 的内容
|
这个PR也添加 utest_auto_run 进来吧，这类测试用例是需要ci去进行持续运行维护的 |
|
@Ryan-CW-Code 另外对于异常参数的检查，其实也包含了一部分，只不过覆盖的可能不是那么全，后续可以补充 |
|
运行截图： ps：这里的断言信息需要该PR合并： sal utestcase[I/utest] [==========] [ utest ] loop 1/1 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
🏷️ Tag: workflowReviewers: Rbb666 kurisaW supperthomas Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-10-23 09:38 CST)
📝 Review Instructions
|
kurisaW
force-pushed
the
testcase
branch
3 times, most recently
from
2dd23ca to
302ee6d
Compare
4 participants
拉取/合并请求描述：(PR description)
添加sal api测试用例覆盖
具体测试内容如下：
覆盖api：