Skip to content

cw.testing: recorded --help bodies can embed the recording user's home directory, silently #38

Closed

Description

@thorwhalen

The problem

cw.testing.characterize records --help bodies verbatim, and a --help body routinely
contains an absolute path under the recording user's home directory — because argparse renders
parameter defaults into the help text, and defaults are often computed from $HOME.

Concrete case from the apps-1 migration wave (thorwhalen/scraped):

$ scraped download-site --help
...
  -r ROOTDIR, --rootdir ROOTDIR
                        '/Users/<user>/.config/scraped/data'

The documented workflow is "Do this before the migration, and commit the result." Following it
literally would have committed a local path into a public repository.

Why it is silent

The case replays fine. --help cases are tier 3, so the body is a snapshot and is not asserted;
replay reports identical on any machine. Nothing fails, nothing warns. The only signal is a
human thinking to grep the golden before git add.

This is the same family as the already-reported prog-absolute-path defect, and the same root
cause: the recorder is faithful to the recording machine, and faithfulness is the wrong default
for a file whose stated destination is a public repo.

What I had to do instead

Drop the case from the golden entirely and hand-roll a usage-line assertion in the test file:

DOWNLOAD_SITE_USAGE = (
    "usage: scraped download-site [-h] [-u [URL_TO_FILEPATH]] [-d DEPTH] "
    "[-f FILTER_URLS] [-m] [-v VERBOSITY] [-r ROOTDIR] url"
)

def test_download_site_grammar_is_unchanged():
    r = subprocess.run(["scraped", "download-site", "--help"], capture_output=True, text=True)
    assert normalise_usage(r.stdout) == normalise_usage(DOWNLOAD_SITE_USAGE)

That is exactly tier 2, re-implemented by hand outside the golden — the information cw already had
and already knew how to compare.

Other cases in the same wave that had to be excluded for the same reason: lookbook list-recipes
(reads YAML profiles from the app data folder), ov runs / ov check, and ov analyze|report| synopsis <missing-run> (tracebacks whose frames are absolute paths).

Suggested fixes, any one of which would have been enough

  1. characterize(..., redact=[(pattern, replacement)]) — applied to recorded stdout/stderr
    before writing. Most general; also covers tracebacks and temp dirs.
  2. A per-case tier cap — e.g. omit_body=True / tier_cap=2, storing the usage line and
    dropping the body. The case stays in the corpus and keeps being asserted at the tier that
    actually carries the grammar.
  3. A warning at record time when a recorded body contains /Users/, /home/, C:\Users, the
    value of $HOME, or the hostname. Cheapest, and it converts a silent trap into a visible one.

(3) alone would be a real improvement: today the failure mode is discovered only by a maintainer
who happens to look.

Related

Prior wave already reported that prog and env are baked in verbatim, with the same
consequence. This is the third field with the same problem, which suggests the fix belongs in
write_golden (one scrub/redact pass over everything it is about to serialise) rather than
per-field.

Found during the apps-1 argh→cw migration wave: reeleehq/lookbook#6, thorwhalen/ov#20,
thorwhalen/citeget#14, thorwhalen/scraped#14.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions