test(bufflog): add a jest suite and a CI workflow - #44
Merged
Merged
Conversation
This package is published to npm and four services import it, and it had no tests and no CI. A broken redact path or a changed level number reaches those services at install time, with nothing in between. The tests mock the pino module so the module-level logger writes to a Writable sink instead of stdout. That keeps the real level config and the real redaction paths in play, and needs no change to bufflog.ts. Coverage is what bufflog.ts promises: the six custom levels and their numeric values, the pino redact paths from constants.ts, and the express middleware over a real HTTP request. One test records that a bare password key in context is not redacted today, because sanitizeContext() returns the context untouched. CI runs build (tsc) and test on node 18 and 20. tsc compiles the tests too, so `files` excludes dist/__tests__ to keep the published tarball at source files only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Comment |
minac
requested review from
brunojppb and
philippemiguet
and removed request for
brunojppb
September 11, 2026 09:22
philippemiguet
approved these changes
Sep 11, 2026
The test asserted that a bare `context.password` reaches the logs in clear text. It recorded the leak rather than a behaviour we want, so it reads as a test for something broken. Its second assertion, that `context.req.headers` is censored, already runs in the test above, which loops every key in `serverKeys` including `headers`. Nothing else is lost. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Jest plus ts-jest, 15 tests over two files, and a
CIworkflow that runsnpm ci,npm run buildandnpm teston node 18 and 20. The tests mock thepinomodule so bufflog's logger writes to aWritableinstead of stdout. That keeps the real level and redact config in play. The middleware tests run a real express server on an ephemeral port.Why
The package had no tests and no CI (the
testscript just exited 1). Every service imports it, and the redaction config is the part nobody wants to break quietly.How
tsconfig.jsonhas noexclude, sotscnow emitsdist/__tests__. I kept it out of the tarball with"!dist/__tests__"infilesinstead of excluding the directory, becausenpm run buildis the regression gate and the tests should go through it.The redaction assertions deserve the closest look, since they encode what counts as a leak. No library code changed.
How to verify
npm cinpm run buildnpm test(2 suites, 15 tests)How does it look
N/A, non-visual.
🤖 Generated with Claude Code