-
Notifications
You must be signed in to change notification settings - Fork 19
test: add logger/aggregator tests for blocked domain detection #1262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -8,10 +8,15 @@ import { LogFormatter } from './log-formatter'; | |||||
| import { LogSource } from '../types'; | ||||||
| import execa from 'execa'; | ||||||
| import { Readable } from 'stream'; | ||||||
| import { trackPidForPortSync, isPidTrackingAvailable } from '../pid-tracker'; | ||||||
|
|
||||||
| // Mock external dependencies | ||||||
| jest.mock('execa'); | ||||||
| jest.mock('fs'); | ||||||
| jest.mock('../pid-tracker', () => ({ | ||||||
| trackPidForPortSync: jest.fn().mockReturnValue({ pid: -1, cmdline: '', comm: '', inode: 0 }), | ||||||
|
||||||
| trackPidForPortSync: jest.fn().mockReturnValue({ pid: -1, cmdline: '', comm: '', inode: 0 }), | |
| trackPidForPortSync: jest.fn().mockReturnValue({ pid: -1, cmdline: '', comm: '', inode: '0' }), |
Copilot
AI
Mar 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These withPid tests exercise PID enrichment while follow: false on a preserved file source. In the CLI, --with-pid is explicitly disabled unless -f/--follow is set (and the option docs say "real-time only"), so this test is asserting behavior that is effectively unreachable and contradicts the intended contract. Consider switching the test to follow: true (tailing) or otherwise aligning it with the documented/CLI behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test name contradicts the asserted behavior: it says "should mark TCP_HIT as allowed" but the test (correctly, per current
isAllowedlogic) expectsisAllowedto be false. Please rename the test to reflect the actual expectation so failures are easier to interpret.