Skip to content

Added two test files prune.test and error.test - #77

Merged
AnupamKumar-1 merged 1 commit into
Obyflow:mainfrom
tarunkumartiwary1:missing-test-for-cli-commands-part1
Sep 10, 2026
Merged

Added two test files prune.test and error.test#77
AnupamKumar-1 merged 1 commit into
Obyflow:mainfrom
tarunkumartiwary1:missing-test-for-cli-commands-part1

Conversation

@tarunkumartiwary1

Copy link
Copy Markdown
Contributor

Added two tests prune.test and error.test.
Fixes : #67

I will file seperate issues and create PR for remaining command tests.

test

all tests passes successfully.

@AnupamKumar-1 AnupamKumar-1 self-assigned this Sep 10, 2026
@AnupamKumar-1

Copy link
Copy Markdown
Member

@tarunkumartiwary1 Nice work on this. The tests are well isolated and cover the main behavior of both commands. I also checked the assertions against the current implementations in errors.ts, prune.ts, sqlite-store.ts, and time.ts, and everything lines up.

A few non-blocking things to consider:

  • expect(output).toContain("2 error(s)") could be a little stricter, e.g. /\b2 error\(s\)/, to avoid accidentally matching "12 error(s)".
  • --limit, --since, and --watch aren't covered yet. That's fine if they're planned for follow-up PRs.
  • It might be worth adding a boundary case for prune where the timestamp is exactly at the cutoff, just to make the < vs <= behavior explicit.
  • buildProgram() and seedEvents() are duplicated between the two test files. With more command tests coming, a shared helper could make sense, but I wouldn't block this PR on it.

Overall this looks good to me. No production changes and the added coverage is useful.

@AnupamKumar-1 AnupamKumar-1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, with some feedback.

@AnupamKumar-1
AnupamKumar-1 merged commit 6555609 into Obyflow:main Sep 10, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing tests for several CLI commands

2 participants