Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughTwo test files updated: Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 |
mabrarov
force-pushed
the
feature/macos_ci
branch
from
7c3a079 to
dd40e9e
Compare
mabrarov
changed the title
mabrarov
force-pushed
the
feature/macos_ci
branch
from
dd40e9e to
cb19720
Compare
mabrarov
force-pushed
the
feature/macos_ci
branch
from
cb19720 to
518b3bf
Compare
mabrarov
force-pushed
the
feature/macos_ci
branch
from
d2bd491 to
bb48cae
Compare
mabrarov
force-pushed
the
feature/macos_ci
branch
from
bb48cae to
479d093
Compare
mabrarov
force-pushed
the
feature/macos_ci
branch
from
ba43828 to
bbb99d9
Compare
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d62489657
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/internal/input_chunk.c (1)
848-848: Line 848: keep this drop-counter test from becoming vacuousWith
storage.total_limit_size = 1M, this test may pass with zero drops while still satisfying current assertions (<= append_countand equality). If the intent is to validate drop counters under pressure, add a positive-drop assertion or lower the limit enough to trigger drops deterministically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/internal/input_chunk.c` at line 848, The test sets "storage.total_limit_size" to "1M" which can allow zero drops and make the drop-counter assertions vacuous; update the test in tests/internal/input_chunk.c to either lower "storage.total_limit_size" (e.g., to a value that will deterministically cause drops given the current append pattern) or add an explicit positive-drop assertion (e.g., require drops > 0) alongside the existing <= append_count/equality checks so the test actually verifies drop behavior; locate the assertions referencing append_count and the drop counter in the same test and adjust the limit or add the positive assertion accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/internal/input_chunk.c`:
- Line 304: The test's configured output limit "storage.total_limit_size" is set
to "1M" but the test asserts total_bytes <= 1000 elsewhere, causing a mismatch;
update the test configuration for "storage.total_limit_size" in
tests/internal/input_chunk.c (the string literal "storage.total_limit_size") to
a value that matches the assertion (e.g., "1000" or "1k") so the drop-pressure
condition and the total_bytes <= 1000 expectation remain consistent.
---
Nitpick comments:
In `@tests/internal/input_chunk.c`:
- Line 848: The test sets "storage.total_limit_size" to "1M" which can allow
zero drops and make the drop-counter assertions vacuous; update the test in
tests/internal/input_chunk.c to either lower "storage.total_limit_size" (e.g.,
to a value that will deterministically cause drops given the current append
pattern) or add an explicit positive-drop assertion (e.g., require drops > 0)
alongside the existing <= append_count/equality checks so the test actually
verifies drop behavior; locate the assertions referencing append_count and the
drop counter in the same test and adjust the limit or add the positive assertion
accordingly.
🪄 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: 4c273f32-527d-48cc-abfe-7c60f340553a
📒 Files selected for processing (2)
tests/internal/input_chunk.ctests/runtime/out_counter.c
| "Port", "1", | ||
| "retry_limit", "no_retries", | ||
| "storage.total_limit_size", "1K", | ||
| "storage.total_limit_size", "1M", |
There was a problem hiding this comment.
Maybe we can remove this line at all:
| "storage.total_limit_size", "1M", |
because the test is not about limit, but is about drop of not delivered chunks.
… behavior Signed-off-by: Marat Abrarov <abrarov@gmail.com>
1 participant
Changes
The tests on macOS where failing on CI like https://github.com/fluent/fluent-bit/actions/runs/23661924165/job/68936596330 (with debug logging turned on):
Testing
TEST_PRESET=valgrind SKIP_TESTS='flb-rt-out_td flb-it-network' ./run_code_analysis.shok-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