Skip to content

Conversation

@mabrarov
Copy link
Contributor

@mabrarov mabrarov commented Jan 22, 2026

Support of Windows in flb-it-input_chunk_routes tests (introduced in #11097).


Testing

  • [N/A] Example configuration file for the change.
  • [N/A] Debug log output from testing the change.
  • Attached Valgrind output that shows no leaks or memory corruption was found - refer to flb_run_code_analysis.log for the output of command
    TEST_PRESET=valgrind SKIP_TESTS='flb-rt-out_td flb-it-network' ./run_code_analysis.sh
  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • [N/A] Documentation required for this feature.

Backporting

  • [N/A] Backport to latest stable release.

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

  • Tests
    • Tests now use per-test temporary directories instead of fixed paths for isolated artifacts.
    • Setup/teardown updated to propagate dynamic paths through test flows, with improved error handling and guaranteed freeing to avoid leftovers and leaks.
  • New Features
    • Added small helper utilities to detect and build temporary directory paths for tests.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

📝 Walkthrough

Walkthrough

Switch tests from hard-coded /tmp paths to environment-aware temporary directories by adding tempdir helper functions, including the helper in internal test headers, and propagating dynamic path allocation, NULL checks, and frees through multiple input-chunk tests and their setup/teardown. (30 words)

Changes

Cohort / File(s) Summary
Tempdir helper
tests/include/flb_tests_tmpdir.h
Add static inline helpers flb_test_env_tmpdir() and flb_test_tmpdir_cat(const char *postfix) that return newly allocated tempdir strings.
Test harness include update
tests/internal/flb_tests_internal.h.in
Include flb_tests_tmpdir.h to expose tempdir utilities to internal tests.
Input chunk tests — dynamic paths
tests/internal/input_chunk_routes.c, tests/internal/input_chunk.c
Replace hard-coded /tmp/TEST_STREAM_PATH constants with dynamically allocated stream_path/storage_path via tempdir helpers; add allocation NULL/error checks; free allocations on error and cleanup; propagate dynamic paths into setup/cleanup calls and test routines.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • edsiper

Poem

🐇 I nibbled paths where /tmp once lay,

I stitched new burrows for each test-day,
I allocate, check, then free with care,
No stray footprints linger anywhere,
Hopping happy in tidy fileplay.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding Windows support for input_chunk_routes tests by introducing platform-independent temporary directory handling.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfa533bbda

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tests/internal/input_chunk_routes.c`:
- Around line 338-356: The env_tmpdir function currently checks TMPDIR then TMP
and falls back to "/tmp", which fails on Windows when TEMP is set but TMP is
not; update env_tmpdir to also check getenv("TEMP") before the fallback and on
Windows use a Windows-appropriate fallback (either call
GetTempPathA()/GetTempPathW when available or use "C:\\Temp") instead of "/tmp",
returning the result via flb_strdup to match existing allocation behavior.
🧹 Nitpick comments (1)
tests/internal/input_chunk_routes.c (1)

20-21: Remove duplicate flb_mem.h include.

<fluent-bit/flb_mem.h> is already included at Line 10, so the Line 20 include is redundant.

♻️ Suggested cleanup
-#include <fluent-bit/flb_mem.h>
 `#include` <fluent-bit/flb_str.h>

@mabrarov mabrarov force-pushed the feature/input_chunk_routes_test branch from cfa533b to 4a292d3 Compare January 22, 2026 12:55
@mabrarov mabrarov force-pushed the feature/input_chunk_routes_test branch from 4a292d3 to 4cfe096 Compare January 25, 2026 10:59
@mabrarov mabrarov force-pushed the feature/input_chunk_routes_test branch from 4cfe096 to 1cf5c24 Compare January 26, 2026 09:52
@mabrarov mabrarov force-pushed the feature/input_chunk_routes_test branch from 1cf5c24 to 49ede29 Compare January 27, 2026 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants