Skip to content

tests: oauth2: Windows support#11506

Merged
edsiper merged 1 commit into
fluent:masterfrom
mabrarov:feature/oauth2_test_windows
Mar 12, 2026
Merged

tests: oauth2: Windows support#11506
edsiper merged 1 commit into
fluent:masterfrom
mabrarov:feature/oauth2_test_windows

Conversation

@mabrarov

@mabrarov mabrarov commented Feb 28, 2026

Copy link
Copy Markdown
Contributor

Support of Windows in tests for OAuth 2.0 (fix of issue introduced in #11485).


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
    • Enhanced OAuth2 test reliability through improved temporary directory management and resource cleanup, preventing potential file conflicts and ensuring consistent test execution across environments.

Signed-off-by: Marat Abrarov <abrarov@gmail.com>
@coderabbitai

coderabbitai Bot commented Feb 28, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Modified the OAuth2 test suite to replace hard-coded temporary file paths with dynamically allocated temporary directories using flb_test_env_tmpdir(). Added proper NULL checks and cleanup logic for both success and error paths, ensuring temporary resources are freed and intermediate files are unlinked on failures.

Changes

Cohort / File(s) Summary
Test Infrastructure Cleanup
tests/internal/oauth2.c
Replaced static /tmp paths with dynamic temporary directory allocation; added NULL validation and cleanup on both success and failure paths; improved resource management with proper file unlinking on errors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • edsiper
  • cosmo0920

Poem

🐰 Files scattered, temp dirs bare,
Now the test cleans up with care!
Dynamic paths dance to and fro,
Cleanup happens high and low! 🧹✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 directly addresses the main change: adding Windows support to OAuth2 tests, which aligns with the core objective of the PR.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/internal/oauth2.c (1)

540-548: ⚠️ Potential issue | 🟡 Minor

Tighten failure cleanup to remove partially created files.

If a write fails after file creation, temp artifacts can remain. Please unlink both targets on their respective failure paths.

🧹 Suggested patch
     ret = write_text_file(key_path, TEST_PRIVATE_KEY_PEM);
     if (ret != 0) {
+        unlink(key_path);
         return -1;
     }

     ret = write_text_file(cert_path, TEST_CERT_PEM);
     if (ret != 0) {
         unlink(key_path);
+        unlink(cert_path);
         return -1;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/internal/oauth2.c` around lines 540 - 548, When write_text_file fails
you must remove any partially created temp files: after the first
write_text_file(key_path, ...) failure call unlink(key_path) before returning,
and after the second write_text_file(cert_path, ...) failure call
unlink(cert_path) as well as unlink(key_path) before returning; update the error
paths in the block handling write_text_file, referencing write_text_file,
key_path, cert_path and using unlink() (ignore unlink errors) so neither
artifact remains on failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@tests/internal/oauth2.c`:
- Around line 540-548: When write_text_file fails you must remove any partially
created temp files: after the first write_text_file(key_path, ...) failure call
unlink(key_path) before returning, and after the second
write_text_file(cert_path, ...) failure call unlink(cert_path) as well as
unlink(key_path) before returning; update the error paths in the block handling
write_text_file, referencing write_text_file, key_path, cert_path and using
unlink() (ignore unlink errors) so neither artifact remains on failure.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a7db0c7 and d7e28c9.

📒 Files selected for processing (1)
  • tests/internal/oauth2.c

@mabrarov mabrarov changed the title tests: internal: oauth2: windows support tests: internal: oauth2: Windows support Feb 28, 2026
@mabrarov mabrarov changed the title tests: internal: oauth2: Windows support tests: oauth2: Windows support Feb 28, 2026
@cosmo0920 cosmo0920 added this to the Fluent Bit v5.0 milestone Mar 2, 2026
@edsiper edsiper merged commit 405fed0 into fluent:master Mar 12, 2026
65 of 66 checks passed
scne59 pushed a commit to scne59/fluent-bit that referenced this pull request May 4, 2026
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
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.

3 participants