tests: internal: Windows support fix, build: SIMD detection fix#11604
tests: internal: Windows support fix, build: SIMD detection fix#11604edsiper merged 2 commits intofluent:masterfrom
Conversation
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
📝 WalkthroughWalkthroughAdded Windows-specific Winsock initialization to test setup ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/internal/task_map.c (1)
42-44: Consider checking the return value ofWSAStartup.
WSAStartupcan fail and returns a non-zero error code on failure. While the subsequentmk_event_loop_createwill likely fail and be caught byTEST_CHECK, explicitly checking the return improves debuggability on Windows.Additionally, for completeness, a matching
WSACleanup()call could be added totest_ctx_destroy. For short-lived test processes this is less critical, so it's optional.♻️ Suggested improvement
`#ifdef` _WIN32 - WSAStartup(0x0201, &wsa_data); + if (WSAStartup(0x0201, &wsa_data) != 0) { + TEST_MSG("WSAStartup failed"); + flb_config_exit(ret_ctx->config); + flb_free(ret_ctx); + return NULL; + } `#endif`And optionally in
test_ctx_destroy:int test_ctx_destroy(struct test_ctx* ctx) { if (!TEST_CHECK(ctx != NULL)) { return -1; } if (ctx->config) { flb_config_exit(ctx->config); } flb_free(ctx); `#ifdef` _WIN32 WSACleanup(); `#endif` return 0; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/internal/task_map.c` around lines 42 - 44, The WSAStartup call in tests/internal/task_map.c is not checked and may fail silently; update the initialization (WSAStartup(...)) to capture and assert its return value (non-zero indicates failure) so tests fail with a clear message, and optionally add a matching WSACleanup() in test_ctx_destroy (or corresponding teardown function) to release Winsock resources; reference the WSAStartup call in the test initialization and the test_ctx_destroy teardown function when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/internal/task_map.c`:
- Around line 42-44: The WSAStartup call in tests/internal/task_map.c is not
checked and may fail silently; update the initialization (WSAStartup(...)) to
capture and assert its return value (non-zero indicates failure) so tests fail
with a clear message, and optionally add a matching WSACleanup() in
test_ctx_destroy (or corresponding teardown function) to release Winsock
resources; reference the WSAStartup call in the test initialization and the
test_ctx_destroy teardown function when making these changes.
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
Changes
Testing
Before we can approve your change; please submit the following in a comment:
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit