Skip to content

scripts: enable working runtime tests#11724

Open
mabrarov wants to merge 4 commits intofluent:masterfrom
mabrarov:feat/http_client_test
Open

scripts: enable working runtime tests#11724
mabrarov wants to merge 4 commits intofluent:masterfrom
mabrarov:feat/http_client_test

Conversation

@mabrarov
Copy link
Copy Markdown
Contributor

@mabrarov mabrarov commented Apr 16, 2026

Summary

Fixed support of runtime tests in HTTP client. Refer to #11686 (comment). This change was already done / merged into master branch in #11710 and was removed from this PR.

Enabled runtime tests for Elasticsearch and Forward output plugins in dev script.

The enabled tests were successfully executed on CI within e37d1f3 temporary commit using fluent/fluent-bit-ci#153 - refer to https://github.com/fluent/fluent-bit/actions/runs/25181471509?pr=11724.


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 ./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

  • Bug Fixes

    • Added a null-pointer safety check to HTTP client connection setup.
  • Tests

    • Reduced the default test skip list so Elasticsearch and forward outputs are included in default test runs.
  • Chores

    • CI unit-test workflow updated to source reusable test scripts from a different repository reference.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Two configuration changes: a shell script's default test exclusion list removes two modules, and a GitHub Actions workflow updates the CI helper checkout to reference a different repository and branch.

Changes

Cohort / File(s) Summary
Build Configuration
run_code_analysis.sh
Default SKIP_TESTS fallback value shortened to exclude flb-rt-out_elasticsearch and flb-rt-out_forward, affecting which CMake modules are disabled by default when the variable is unset.
CI/CD Pipeline
.github/workflows/unit-tests.yaml
CI helper checkout step updated to source from mabrarov/fluent-bit-ci instead of calyptia/fluent-bit-ci and pinned to feat/enable_tests branch in Ubuntu and macOS unit-test jobs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • edsiper
  • cosmo0920

Poem

🐰 Two modules skip no more—a lighter test,

CI checkouts shift to branches fresh and blessed,

With faster runs and sources newly placed,

This tiny diff keeps workflows well-graced! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title 'scripts: enable working runtime tests' is directly related to the main changes: modifying run_code_analysis.sh to adjust SKIP_TESTS defaults and updating CI workflow configuration to use custom CI scripts with enabled runtime tests.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@mabrarov mabrarov force-pushed the feat/http_client_test branch from c86abe0 to 31424a8 Compare April 16, 2026 17:17
@mabrarov mabrarov force-pushed the feat/http_client_test branch from 76edf7c to f006d83 Compare April 16, 2026 21:44
Copy link
Copy Markdown

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/unit-tests.yaml:
- Around line 116-117: In the unit-test workflow jobs that currently reference
the personal fork (the repository: mabrarov/fluent-bit-ci and ref:
feat/enable_tests entries in the unit-test job), revert those two fields to the
official CI repository and branch used elsewhere (e.g., repository:
fluent/fluent-bit-ci and the stable ref previously used) so both Linux and macOS
unit-test jobs match the other workflows (pr-perf-test.yaml and
call-run-integration-test.yaml); update the repository and ref values in the
unit-test job definitions to remove the personal fork and feature branch.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 434bfef2-66e6-4d3f-a46e-8e3cb76877ee

📥 Commits

Reviewing files that changed from the base of the PR and between 2ebb0e7 and cd068ee.

📒 Files selected for processing (2)
  • .github/workflows/unit-tests.yaml
  • run_code_analysis.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • run_code_analysis.sh

Comment thread .github/workflows/unit-tests.yaml Outdated
@cosmo0920
Copy link
Copy Markdown
Contributor

We need to use shorter than 80 characters commit messages:


❌ Commit cd068ee223 failed:
Commit subject too long (>80 chars): 'workflows: temporarily switched to CI scripts with enabled runtime tests for Forward and Elasticsearch output plugins, for Disk I/O metrics and Process metrics input plugins.'

❌ Commit 9664979ccd failed:
Commit subject too long (>80 chars): 'scripts: enabled runtime tests for Forward output plugin in dev code analysis script'


Commit prefix validation failed.
Error: Process completed with exit code 1.

cosmo0920
cosmo0920 previously approved these changes Apr 21, 2026
… script

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

Signed-off-by: Marat Abrarov <abrarov@gmail.com>
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
Copy link
Copy Markdown

@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.

♻️ Duplicate comments (1)
.github/workflows/unit-tests.yaml (1)

116-117: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Revert CI helper checkout to the project-owned fluent-bit-ci repo/ref

Line 116 and Line 173 still pin CI helper scripts to a personal fork branch (mabrarov/fluent-bit-ci@feat/enable_tests). This is mutable and can break or unexpectedly alter CI behavior for master. Please switch both jobs back to the org-owned/stable CI helper reference before merge.

Suggested change
       - uses: actions/checkout@v6
         with:
-          repository: mabrarov/fluent-bit-ci
-          ref: feat/enable_tests
+          repository: fluent/fluent-bit-ci
+          ref: main
           path: ci
@@
       - uses: actions/checkout@v6
         with:
-          repository: mabrarov/fluent-bit-ci
-          ref: feat/enable_tests
+          repository: fluent/fluent-bit-ci
+          ref: main
           path: ci

Also applies to: 173-174

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/unit-tests.yaml around lines 116 - 117, The CI workflow
still checks out the helper actions from a personal fork (repository:
mabrarov/fluent-bit-ci, ref: feat/enable_tests) at the two places using those
keys; update both occurrences (the repository: and ref: entries around the
checkout steps at the two job blocks) to point back to the org-owned stable
helper (e.g., repository: fluent-bit/fluent-bit-ci and the stable ref or tag
used across the project) so both jobs use the official CI helper repo/ref before
merging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In @.github/workflows/unit-tests.yaml:
- Around line 116-117: The CI workflow still checks out the helper actions from
a personal fork (repository: mabrarov/fluent-bit-ci, ref: feat/enable_tests) at
the two places using those keys; update both occurrences (the repository: and
ref: entries around the checkout steps at the two job blocks) to point back to
the org-owned stable helper (e.g., repository: fluent-bit/fluent-bit-ci and the
stable ref or tag used across the project) so both jobs use the official CI
helper repo/ref before merging.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eaf394f9-85a3-451f-8180-af078f5657e2

📥 Commits

Reviewing files that changed from the base of the PR and between 099bb5c and e37d1f3.

📒 Files selected for processing (2)
  • .github/workflows/unit-tests.yaml
  • run_code_analysis.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • run_code_analysis.sh

Copy link
Copy Markdown
Contributor

@patrick-stephens patrick-stephens left a comment

Choose a reason for hiding this comment

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

Seems a good idea to do more.

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