You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
common/src/__tests__/freebuff-public-data-use-copy.test.ts cannot pass in this
repository. On current main (2d10aaa8d) it reports 1 pass, 5 fail. This PR
makes it 4 pass, 3 skip, 0 fail, test-only, in one file.
Why it fails
Two independent causes, both specific to this repository being a public export of
a private source tree.
1. Line endings decide the comparison (2 failures).README.md and freebuff/cli/release/README.md failed with a diff of - Expected - 0 / + Received + 0 — zero changed lines, i.e. an invisible difference. The cause is
one stray \r per line:
$ git show HEAD:README.md # the committed blob itself
blob CRLF: 117 LF: 117
$ # inside the generated block only
block CR count: 8 block LF count: 8
The blob in git is CRLF, there is no .gitattributes pinning either style, and renderFreebuffDataUseFaqMarkdown() produces an LF-only template literal. So the
block in the file cannot equal the generated copy byte-for-byte on any platform —
this is not a Windows checkout artifact, and the block has 8 CR for its 8 LF. The
assertion was testing whoever last saved the file, not the copy it is named for.
2. Three cases read files this repository does not ship (3 failures). Plain ENOENT:
error: ENOENT: no such file or directory, open '.../web/src/content/advanced/privacy.mdx'
error: ENOENT: no such file or directory, open '.../web/src/content/help/faq.mdx'
error: ENOENT: no such file or directory, open '.../landing-lab/src/components/sections/Faq.tsx'
CONTRIBUTING.md describes this repository as a public mirror, and pr-hygiene.yml lists web/ among the paths that "are not part of this
repository". Those cases can never pass here, so they fail rather than skip.
What this changes
readRepoFile normalizes CRLF (and stray lone CR) to LF, so the assertion
compares the copy's words.
The test.each table becomes per-case test.skipIf(!isInThisTree(path)), using
the test.skipIf convention already present in this repo. In the private tree,
where web/ and landing-lab/ do exist, all five cases run unchanged; if the
export ever grows to include them, the cases reactivate on their own with no
edit here.
One new test, line endings do not decide whether the copy matches, locks the
normalization. It is not a tautology: with a no-op normalizer, normalizeLineEndings(copy.replace(/\n/g, '\r\n')) returns the CRLF string and
fails against the LF-only renderer.
How it was tested
$ bun test src/__tests__/freebuff-public-data-use-copy.test.ts
before (main @ 2d10aaa8d)
after
result
1 pass, 5 fail
4 pass, 3 skip, 0 fail
The two README cases are the regression lock — verified failing on unmodified main before the change, passing after. Surrounding suites on the same base,
unaffected by this diff:
bun test src/util/__tests__ 739 pass, 0 fail (58 files)
bunx prettier --check common/src/__tests__/freebuff-public-data-use-copy.test.ts
All matched files use Prettier code style!
Notes for the reviewer
Alternative to the line-ending half: converting the committed README.md and freebuff/cli/release/README.md to LF would also make the test pass, but that is
a ~117-line whitespace change to two docs files and it would recur for the next
person who saves with CRLF. Happy to switch to that approach if you would rather
the bytes be canonical — it is a one-line change to this PR.
Why this was never caught:Public CI (.github/workflows/ci.yml) installs,
builds the SDK, builds the binary and smoke-tests it. It never runs bun test, so
a red suite in the exported tree is invisible to CI. I am not proposing a CI
change here, but this is the second file I found in the same situation and it may
be worth a separate look at whether common/ tests should run in public CI.
common/src/__tests__/free-agents.test.ts has the same cause 2, for three
prompt-opening cases reading freebuff/web/convex/... and two freebuff-desktop/... files. I kept that out of this diff to keep it to one
thing; happy to send it as a follow-up.
No source behavior is touched, and no web/, freebuff/web/, packages/internal/, packages/billing/, packages/bigquery/ or packages/build-tools/ path is modified.
Good instinct and solid execution for a narrow problem: freebuff-public-data-use-copy.test.ts can't pass here because (a) README.md carries CRLF while the renderer emits LF-only text, and (b) three cases read web//landing-lab/ paths this mirror doesn't ship. Your fix — normalize line endings in readRepoFile, and test.skipIf(!isInThisTree(path)) per case — is minimal and doesn't change behavior in a tree where those files exist, since isInThisTree would just return true there. The new regression test for the normalizer is a reasonable guard, not a tautology.
Two things worth reconsidering before this gets ported:
Normalizing in the reader treats CRLF-vs-LF as a permanent fact of life rather than asking why README.md is committed as CRLF with no .gitattributes pinning a style. If that's an export artifact, a .gitattributes entry (README.md text eol=lf) fixing it at the source is more durable than normalizing forever in one test file — and it would also stop any future diff noise from unrelated line-ending flips.
Converting test.each to a manual for loop changes the test's reporting shape (name, parametrization) for a fairly small reason — test.each supports conditional skip via a similar pattern, so it's worth checking if you can keep the table form and just gate the assertion body instead. Not a blocker, just a style nit a maintainer may push back on.
The PR is well-scoped (one file, test-only) and the before/after numbers are concrete and checkable. Worth a maintainer's look, possibly with the .gitattributes alternative folded in.
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
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.
common/src/__tests__/freebuff-public-data-use-copy.test.tscannot pass in thisrepository. On current
main(2d10aaa8d) it reports 1 pass, 5 fail. This PRmakes it 4 pass, 3 skip, 0 fail, test-only, in one file.
Why it fails
Two independent causes, both specific to this repository being a public export of
a private source tree.
1. Line endings decide the comparison (2 failures).
README.mdandfreebuff/cli/release/README.mdfailed with a diff of- Expected - 0/+ Received + 0— zero changed lines, i.e. an invisible difference. The cause isone stray
\rper line:The blob in git is CRLF, there is no
.gitattributespinning either style, andrenderFreebuffDataUseFaqMarkdown()produces an LF-only template literal. So theblock in the file cannot equal the generated copy byte-for-byte on any platform —
this is not a Windows checkout artifact, and the block has 8 CR for its 8 LF. The
assertion was testing whoever last saved the file, not the copy it is named for.
2. Three cases read files this repository does not ship (3 failures). Plain
ENOENT:CONTRIBUTING.mddescribes this repository as a public mirror, andpr-hygiene.ymllistsweb/among the paths that "are not part of thisrepository". Those cases can never pass here, so they fail rather than skip.
What this changes
readRepoFilenormalizes CRLF (and stray lone CR) to LF, so the assertioncompares the copy's words.
test.eachtable becomes per-casetest.skipIf(!isInThisTree(path)), usingthe
test.skipIfconvention already present in this repo. In the private tree,where
web/andlanding-lab/do exist, all five cases run unchanged; if theexport ever grows to include them, the cases reactivate on their own with no
edit here.
line endings do not decide whether the copy matches, locks thenormalization. It is not a tautology: with a no-op normalizer,
normalizeLineEndings(copy.replace(/\n/g, '\r\n'))returns the CRLF string andfails against the LF-only renderer.
How it was tested
main@2d10aaa8d)The two README cases are the regression lock — verified failing on unmodified
mainbefore the change, passing after. Surrounding suites on the same base,unaffected by this diff:
Notes for the reviewer
README.mdandfreebuff/cli/release/README.mdto LF would also make the test pass, but that isa ~117-line whitespace change to two docs files and it would recur for the next
person who saves with CRLF. Happy to switch to that approach if you would rather
the bytes be canonical — it is a one-line change to this PR.
Public CI(.github/workflows/ci.yml) installs,builds the SDK, builds the binary and smoke-tests it. It never runs
bun test, soa red suite in the exported tree is invisible to CI. I am not proposing a CI
change here, but this is the second file I found in the same situation and it may
be worth a separate look at whether
common/tests should run in public CI.common/src/__tests__/free-agents.test.tshas the same cause 2, for threeprompt-opening cases reading
freebuff/web/convex/...and twofreebuff-desktop/...files. I kept that out of this diff to keep it to onething; happy to send it as a follow-up.
web/,freebuff/web/,packages/internal/,packages/billing/,packages/bigquery/orpackages/build-tools/path is modified.