Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,10 @@ types/

# Donor/reference codebase kept out of the active HypAware repo
collectivus/

# Tool transcripts: install, test, and typecheck output redirected to a file
# while working. Never source, and nothing else in the toolchain objects to one
# (they are outside the package `files` allowlist and touch no code path), so a
# `git add -A` used to be enough to commit one. `test/core/repo-scratch-hygiene.test.js`
# holds the other half of this: no `.log` is tracked.
*.log
122 changes: 122 additions & 0 deletions test/core/repo-scratch-hygiene.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// @ts-check

// Triage of PR #785 found an `npm-install.log` committed at the repo root: a
// reviewer's install transcript, 11 lines ending `INSTALL EXIT=0`, swept up by
// a `git add -A`. Nothing in the toolchain objected. It is excluded from the
// published file set (`package.json` `files`), so `npm pack` stays clean; it
// touches no code path, so no test moved; and `.gitignore` carried no rule for
// tool transcripts, so `git status` listed it as an ordinary new file. The only
// thing that caught it was a human reading the diff, which is exactly the check
// that is not there next time.
//
// This is the gate for both halves of that, per issue #786: the tree carries no
// tool transcript today, and `.gitignore` refuses one tomorrow. The second half
// is the one that matters - a lint that only notices after the fact still needs
// someone to run it on the right branch, while an ignore rule means the file
// never reaches `git add` in the first place.
//
// It is a lint on a property of the repository rather than a behavior check, in
// the shape of `house-style-em-dash.test.js`.

import test from 'node:test'
import assert from 'node:assert/strict'
import path from 'node:path'
import { execFileSync, spawnSync } from 'node:child_process'
import { fileURLToPath } from 'node:url'

const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..')

/** @returns {string[]} repo-relative paths of every tracked file */
function trackedFiles() {
const out = execFileSync('git', ['ls-files', '-z'], { cwd: REPO_ROOT, encoding: 'utf8' })
return out.split('\0').filter(f => f !== '')
}

/**
* Which ignore rule, if any, would keep `relPath` out of a commit.
*
* `git check-ignore` exits 0 when a path is ignored and 1 when it is not, so
* the status is the answer and exit 1 is not a failure to report. Anything else
* is a real git failure (a `safe.directory` refusal, a checkout with no `.git`)
* and is raised rather than collapsed into "not ignored", which would fail the
* rule test below with a message sending the reader to fix a `.gitignore` that
* is already correct.
*
* `--no-index` is passed so the answer is about the ignore *rules* alone. By
* default `check-ignore` consults the index first and reports any tracked path
* as not ignored, whatever `.gitignore` says. That is the wrong question here,
* and it fails in the one case this file exists for: once a `.log` is tracked,
* the rule probe would flip to "unguarded" and send the reader off to add a
* `.gitignore` rule that is already there. Tracked transcripts are the other
* test's job, and its message names the fix (delete the file).
*
* The matching rule's *source* is returned, not just a yes/no, because the whole
* ignore stack answers this question and only one layer of it is the repo's.
* `*.log` is common in a personal `core.excludesFile` (it ships in widely copied
* global templates), and `.git/info/exclude` is per-clone too. Either would
* stand in for the committed rule, so a `.gitignore` that had lost it would
* still read green on that machine while every fresh checkout was unguarded.
* The global file is pinned to `/dev/null` here and the source is asserted
* below, so only the committed `.gitignore` can satisfy the gate.
*
* @param {string} relPath
* @returns {{ ignored: boolean, source: string, pattern: string }}
*/
function ignoreRule(relPath) {
// `-z` output requires `--stdin`, and it is worth the stdin round trip: the
// default format is `source:line:pattern\tpath`, which is ambiguous for any
// source or pattern containing a colon.
const args = ['-c', 'core.excludesFile=/dev/null', 'check-ignore', '-v', '-z', '--no-index', '--stdin']
const result = spawnSync('git', args, { cwd: REPO_ROOT, input: `${relPath}\0`, encoding: 'utf8' })
if (result.error) throw result.error
if (result.status !== 0 && result.status !== 1) {
throw new Error(`git check-ignore exited ${result.status}: ${(result.stderr || '').trim()}`)
}
if (result.status !== 0) return { ignored: false, source: '', pattern: '' }
const [source, , pattern] = result.stdout.split('\0')
return { ignored: true, source, pattern }
}

test('no tool transcript is tracked in the repo', () => {
const found = trackedFiles().filter(f => f.endsWith('.log'))
assert.deepEqual(found, [], 'a `.log` file is a tool transcript, not source; ' +
`delete it and let \`.gitignore\` hold the line:\n ${found.join('\n ')}`)
})

test('.gitignore refuses a tool transcript', () => {
// The root case is the one that actually happened; the nested cases prove the
// rule is not anchored to the root, because scratch lands in subdirectories
// at least as often as it lands beside `package.json`.
const probes = [
'npm-install.log',
'npm-test.log',
'typecheck.log',
'x/npm-test.log',
'src/core/cli/debug.log',
]
const unguarded = []
for (const probe of probes) {
const rule = ignoreRule(probe)
if (!rule.ignored) unguarded.push(`${probe} (no rule matches)`)
else if (rule.source !== '.gitignore') unguarded.push(`${probe} (matched by ${rule.source}, which is not committed)`)
}
assert.deepEqual(unguarded, [], 'these paths would be committable by a stray ' +
`\`git add -A\`; \`.gitignore\` needs a rule covering them:\n ${unguarded.join('\n ')}`)
})

test('the probe distinguishes ignored from unignored paths', () => {
// A probe that answered "ignored" for everything would pass the rule above
// forever, including on a repo with no `.gitignore` at all. Under
// `--no-index` these read the rules and nothing else, so neither direction
// can be satisfied by the path's tracked status.
assert.equal(ignoreRule('package.json').ignored, false,
'expected a source file no rule matches to read as not ignored')
// The other direction, deliberately not borrowed from an unrelated rule such
// as `*.tgz`: a probe stuck on "ignored" would have to name this file's own
// rule to get here, and a maintainer reorganizing pack output cannot redden a
// transcript-hygiene test.
assert.equal(ignoreRule('npm-install.log').pattern, '*.log',
'expected the transcript rule itself to be the one doing the work')
const files = trackedFiles()
assert.ok(files.length > 500, `expected the tracked tree, found ${files.length} files`)
})
Loading