Skip to content

JAR-430: wire make test and cover the two parsing-heavy files - #52

Open
Jartans-Familiar wants to merge 2 commits into
mainfrom
agent/ferro/c4a8e65f
Open

JAR-430: wire make test and cover the two parsing-heavy files#52
Jartans-Familiar wants to merge 2 commits into
mainfrom
agent/ferro/c4a8e65f

Conversation

@Jartans-Familiar

Copy link
Copy Markdown
Member

Lands the test gate decided in #34 and specified in CONTRIBUTING.md's ## Tests, plus the first two suites behind it. First of two: the hook-script suites follow as a stacked PR, purely additive.

Both languages in one change, because each half of make test asserts its file list is non-empty -- a one-language first PR would leave the other half red.

The gate

Makefile gains a test target: two recipe lines, Node then Python, each building an explicit file list with git ls-files and refusing to run on an empty one. git ls-files exits 0 on no match and node --test with no arguments walks the whole tree instead of failing, so an unguarded list is a gate that reports success having checked nothing. The assertion idiom is the JSON check's, in verify.

.github/workflows/ci.yml gains a test job pinning Node to 20 (the floor the hook scripts have to run on, not the runner's default) and Python to 3.12 as the other jobs do. The check aggregator names test in both places -- the needs: list and the results array -- which is the footgun that file warns about: miss the array and the required status check goes green while the suite is red.

The suites

tests/plugins/praxis/skills/code-hygiene/find-duplicate-comments.test.js -- 26 tests over parseArgs, buildSkipMatcher, prose, trackedPaths, buildCommentIndex, findRetoldInDiff and dedupePairs. The two rejection paths in parseArgs call process.exit, so they are exercised as a spawned process rather than an import.

tests/scripts/test_generate_codex.py -- 40 tests over parse_frontmatter, basic_string, render_agent_toml (including the ''' rejection), display_name, short_description, qualify, plugin_manifest's name-mismatch raise, and both drift finders. The module is loaded with importlib.util.spec_from_file_location, since the hyphen in its filename is not a valid module name.

Fixtures are throwaway repositories built with git init under the OS temp directory, and temp directories with ROOT patched to point at them. Never this checkout -- a fixture that is this repository lets a rule which suppresses a finding here look correct.

Regression tests

Four defects find-duplicate-comments.js accrued over the #33 review rounds were held only by a code comment. Each now has a test named regression:, and each was confirmed to fail with its fix reverted and nothing else:

Test Reverting
a CRLF line still reads as a comment (+ the committed-blob case) the \r strip -> 2 failures
a --skip value is matched literally, not compiled as a pattern escapeRegExp -> 1 failure
the no-newline marker does not advance the line counter the \ guard -> 1 failure
trackedPaths drops a submodule gitlink the mode-160000 filter -> 1 failure

One correction to the record on the gitlink case. find-duplicate-comments.js's header comment says a gitlink fed to cat-file --batch "answers <sha> submodule, which would abort the whole batch." That is current git's documented behaviour but not git 2.43's, which answers missing when the gitlink's commit is absent locally and <sha> commit <size> when it is present -- neither of which aborts. So the test pins the version-independent contract instead: the entry never reaches the batch. The filter is right either way, and on 2.43 the pre-fix cost is a commit object's bytes indexed as if they were source. No code change; the test comment records it.

Seam

find-duplicate-comments.js exports two more functions, parseArgs and trackedPaths, both consumed by the suite -- that is the praxis 1.4.0 -> 1.4.1 bump, and the regenerated .codex-plugin/plugin.json alongside it.

Also

CONTRIBUTING.md and CLAUDE.md drop their "once the suite lands" clauses for the target itself.

Verification

make test (26 Node + 40 Python, all pass) and make verify pass locally. make lint passes every hook that can run here; the three container-based hooks (lychee, actionlint, zizmor) cannot -- Docker's socket is not reachable from this runtime -- so the lint job is what covers them. The workflow addition reuses the existing jobs' shapes: SHA-pinned actions and persist-credentials: false.

Closes JAR-430 is deliberately absent -- the issue also carries PR 2.

Refs #34

🤖 Generated with Claude Code

…e65f)

Implements the convention decided in CONTRIBUTING.md's Tests section: the
gate itself, plus the first two suites behind it. Both languages, because
each half of `make test` asserts its file list is non-empty and a
one-language first PR would leave the other half red.

Four of the Node tests are named `regression:` and each pins a defect the
duplicate-comment finder accrued over the grimoire#33 review rounds, until
now held only by a code comment. Each was confirmed to fail with its fix
reverted.

The duplicate-comment seam widens by two functions the suite consumes,
parseArgs and trackedPaths, which is the praxis version bump.

Co-authored-by: multica-agent <github@multica.ai>
import json
import pathlib
import tempfile
import unittest
…5f) (#54)

Second of two, purely additive: no Makefile or CI change, since PR 1 wired
the gate. Adds 113 Node tests over the token accounting and setting
resolvers in lib/transcript-context.js, the sweep and session-id sanitising
in lib/session-state.js, the cooldown split in check-console-log.js, and
the regex-flag and glob handling in lib/utils.js.

Process contracts for all four shipped hook entry points: exit 0 and either
no stdout or exactly one well-formed JSON object, whatever arrives on stdin.
sync-codex-agents.js is spawned rather than imported because hooks.json
invokes it via require and it works at require time, so a require.main
guard would make the hook a no-op; both plugins' copies are covered.

The seam widens by three functions the suite consumes -- readFileTail and
extractUsageTokens in transcript-context, filterByPatterns in utils --
which is the praxis version bump.

Co-authored-by: multica-agent <github@multica.ai>
});

test('applyCooldown stops decrementing at zero', () => {
const { report, state } = applyCooldown([A], { seen: [A.key], cooldown: 0 }, COOLDOWN);
@JartanFTW

Copy link
Copy Markdown
Contributor

resolve conflicts

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.

2 participants