diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100755 index 0000000..8b13789 --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1 @@ + diff --git a/tests/test_apply_records_application.py b/tests/test_apply_records_application.py new file mode 100755 index 0000000..bff4592 --- /dev/null +++ b/tests/test_apply_records_application.py @@ -0,0 +1,479 @@ +"""Guards for /apply's tracker recording step (Step 6b). + +The step is part of the /apply markdown spec (the spec IS the +implementation), so these tests pin the invariants that would break +silently. Assertions are scoped to the section they belong to, following +the pattern in test_upskill_skill.py: a whole-file `assertIn` for a word +as common as `drafted` passes on any unrelated mention and guards nothing. + +The CSV header is the one rule most easily lost: it must stay +byte-identical to /outcome's, which is the entire reason for reusing it. +How each reader treats `drafted` is pinned per reader below, because the +right answer differs between them. +""" +import re +import subprocess +import sys +import unittest +from pathlib import Path + +try: + import yaml # noqa: F401 - only probing availability for the lint integration test + _HAVE_YAML = True +except ImportError: + _HAVE_YAML = False + +REPO = Path(__file__).resolve().parent.parent +COMMANDS = REPO / ".claude" / "commands" +APPLY = COMMANDS / "apply.md" +OUTCOME = COMMANDS / "outcome.md" +GMAIL_SYNC = COMMANDS / "gmail-sync.md" +HTML_REPORT = COMMANDS / "html-report.md" +INTERVIEW = COMMANDS / "interview.md" +NOTION_SYNC = COMMANDS / "notion-sync.md" +SKILL = REPO / ".claude" / "skills" / "job-application-assistant" / "SKILL.md" +SCRAPER = REPO / ".claude" / "skills" / "job-scraper" / "SKILL.md" +DOCS_README = REPO / "documents" / "README.md" + +TRACKER_HEADER = ( + "date,company,sector,role,role_type,channel,status,contact_person," + "fit_rating,notes,cv_file,cover_letter_file,source,deadline" +) + + +def section(path, heading): + """The body of one markdown section, up to the next heading of any depth.""" + text = path.read_text(encoding="utf-8") + start = text.index(heading) + len(heading) + rest = text[start:] + end = re.search(r"^#{1,4} ", rest, re.MULTILINE) + return rest[: end.start()] if end else rest + + +class ApplyRecordsApplication(unittest.TestCase): + """/apply Step 6b writes the row that six other commands read.""" + + def setUp(self): + self.step_6b = section(APPLY, "### Step 6b: Record the Application") + + def test_step_writes_a_drafted_row_with_both_document_paths(self): + for fragment in ( + "| `status` | `drafted` |", + '| `cv_file`, `cover_letter_file` | the two paths listed under "Files Created"', + ): + self.assertIn( + fragment, + self.step_6b, + f"Step 6b's column table lost {fragment!r} - the row it writes would " + "no longer identify itself as a draft or point at the documents", + ) + + def test_tracker_header_matches_outcome(self): + """Byte-identical, or the two commands create incompatible CSVs. + + The exact-equality loop below is load-bearing, not decoration. `assertIn` + on its own cannot see an *additive* drift: a 13-column header is a + substring of a 14-column one, so appending a column to `/apply` and + forgetting `/outcome` passed this test cleanly until the loop was added. + + It is also what makes the constant-only assertions in this class mean + anything: they reason about TRACKER_HEADER, and this is the test that + anchors TRACKER_HEADER to what both spec files actually say. + """ + self.assertIn(TRACKER_HEADER, OUTCOME.read_text(encoding="utf-8")) + self.assertIn( + TRACKER_HEADER, + self.step_6b, + "Step 6b's header drifted from outcome.md's - whichever command ran " + "first would decide the schema", + ) + for name, text in (("outcome.md", OUTCOME.read_text(encoding="utf-8")), + ("apply.md Step 6b", self.step_6b)): + header = next( + (ln.strip() for ln in text.splitlines() if ln.strip().startswith("date,company,")), + None, + ) + self.assertEqual( + header, + TRACKER_HEADER, + f"{name}'s header line is not exactly the canonical header - a column " + "appended to one file and not the other leaves both containing the " + "shorter header as a substring, which assertIn alone cannot catch", + ) + + def test_tracker_header_ends_with_deadline(self): + """/apply appends rows with one field per header column, so inserting + `deadline` anywhere but the end shifts every value in every existing + row by one position.""" + self.assertTrue( + TRACKER_HEADER.endswith(",deadline"), + "deadline must be the last column - a mid-header insert shifts every " + "existing row's values by one position", + ) + + def test_migration_appends_the_headers_own_last_column(self): + """The migration sentence and the create path must name the same column. + + Derived, never copied - the same discipline `HtmlReportTrackerFieldTests` + already applies to its `CANONICAL_HEADER`. A hardcoded `,deadline` here + keeps passing after the column is renamed or a fifteenth is appended, + because the assertion no longer has any connection to the header it is + supposed to police. A tracker migrated by these commands and one they + create from scratch would then hold different schemas, which is the exact + divergence the shared-header rule exists to prevent. + """ + last_column = TRACKER_HEADER.rsplit(",", 1)[1] + outcome_step_1 = section(OUTCOME, "## Step 1: Load State and Identify the Application") + for name, text in ( + ("apply.md Step 6b", section(APPLY, "### Step 6b: Record the Application")), + ("outcome.md Step 1", outcome_step_1), + ): + self.assertIn( + f"append `,{last_column}` to the header line", + text, + f"{name}'s migration does not append the header's own last column " + f"({last_column!r}) - a tracker migrated by this command would not " + "match one this command creates from scratch", + ) + + def test_step_runs_before_the_optional_offer_that_ends_the_turn(self): + """The optional application-form offer asks the user a question. + + Anything after it only runs if the user answers, so recording the + application there would reproduce the bug this step fixes. + """ + text = APPLY.read_text(encoding="utf-8") + self.assertLess( + text.index("### Step 6b: Record the Application"), + text.index("### Application-Form Fields"), + "Step 6b moved after the optional-artifact offer, which ends the turn " + "on a question - the tracker row would be skipped whenever the user " + "never answers", + ) + + def test_matched_row_is_never_moved_backwards(self): + self.assertIn( + "never move it backwards", + self.step_6b, + "Step 6b lost the rule protecting a submitted row - re-running /apply " + "to refresh a CV would reset a live interview back to drafted", + ) + + def test_redraft_marker_is_undated(self): + """/outcome reads the latest dated note as the last activity.""" + self.assertIn( + "undated `redrafted` marker", + self.step_6b, + "a dated redraft marker resets /outcome's days-quiet clock, hiding a " + "genuinely quiet application from the follow-up offer", + ) + + def test_seen_jobs_is_left_alone(self): + self.assertIn( + "Do not modify `job_scraper/seen_jobs.json`", + self.step_6b, + "drafting is not applying, and that file has no honest value for either", + ) + + def test_skill_defers_to_apply_rather_than_restating(self): + """/scrape Step 5 routes into the skill, bypassing /apply entirely.""" + step_3b = section(SKILL, "### Step 3b: Record the Application") + self.assertIn( + "`/apply` Step 6b", + step_3b, + "the skill's recording step no longer points at the canonical rule, so " + "the two copies can drift", + ) + + +class DraftedMeansDraftedToEveryReader(unittest.TestCase): + """`drafted` is non-final, so readers that mean *submitted* must say so. + + Each of these defines its set by exclusion from the final statuses, so + a new non-final value joins them all silently. The one exception is + /gmail-sync, which must keep searching for drafted rows: the user + submitting by hand and not running /outcome is the failure #269 is + about, and an employer reply is how it gets caught. + """ + + CASES = [ + (HTML_REPORT, None, "`drafted` → **Drafted**", + "a status with no bucket is dropped from every statistic"), + (HTML_REPORT, "## Step 2: Compute Summary Stats", + "excluded from every statistic below", + "the headline count would include applications that were never sent"), + (OUTCOME, "## Step 2b: Follow-Up Branch", "neither final nor `drafted`", + "it would chase an employer who received nothing"), + (OUTCOME, "## Step 4: Update the Tracker", + "overwrite its `date` column with the actual submission date", + "the drafting date would be reported as the application date"), + (GMAIL_SYNC, None, "`drafted` rows stay in this set", + "excluding them discards the row that identifies a submitted-but-" + "unrecorded application, which is the recovery #269 asks for"), + (GMAIL_SYNC, "## Step 5", "`drafted` -> `applied`, otherwise", + "the acknowledgement is the one email that proves a hand-submitted " + "application was sent; classified as noise, the recovery never fires"), + (GMAIL_SYNC, "### Step 7a", "also set `date` to the email's date", + "the row would keep the drafting date after being proved submitted"), + (GMAIL_SYNC, "## Step 9: Staleness Check", "Skip `drafted` rows here", + "an unsent draft reported as a forgotten application"), + (NOTION_SYNC, None, "omit when the status is `drafted`", + "an 'Applied on' date for a job never applied to"), + (NOTION_SYNC, None, "not yet submitted", + "page bodies are write-once, so calling drafts 'submitted documents' " + "is permanent even after /outcome records the real submission"), + (SCRAPER, None, "do not add a second row", + "/scrape would duplicate the row Step 3b just wrote"), + (APPLY, "### Step 6b: Record the Application", "bare number, 0-100", + "/upskill divides by fit_rating, so `72/100` or a verdict word breaks it"), + (APPLY, "### Step 6b: Record the Application", "append a new row", + "re-applying after a rejection would overwrite the old application"), + ] + + def test_every_reader_handles_drafted(self): + for path, heading, needle, why in self.CASES: + with self.subTest(file=path.name, rule=needle): + haystack = section(path, heading) if heading else path.read_text(encoding="utf-8") + self.assertIn(needle, haystack, why) + + @unittest.skipUnless( + _HAVE_YAML, + "PyYAML not installed (the CI Python-test job omits it; the lint job runs lint_skills.py directly)", + ) + def test_lint_skills_passes(self): + result = subprocess.run( + [sys.executable, str(REPO / "tools" / "lint_skills.py")], + cwd=REPO, + capture_output=True, + text=True, + ) + self.assertEqual(result.returncode, 0, f"lint_skills.py failed:\n{result.stdout}{result.stderr}") + + +class ApplyArchivesThePosting(unittest.TestCase): + """Step 6b must also write the posting text it is holding to the archive.""" + + CASES = [ + (APPLY, "## Step 0: Parse Input", + "full posting text verbatim", + "by Step 6b the model may hold only a summary, so the archive gets a " + "paraphrase - what /outcome Step 3.2 forbids"), + (APPLY, "### Step 6b: Record the Application", + "`documents/applications/_/job_posting.md`", + "the one moment /apply provably holds the posting is spent again, and " + "a pasted posting has no recovery path at all"), + (APPLY, "### Step 6b: Record the Application", + "never a fresh fetch", + "a model that no longer holds the text would re-fetch to comply, the " + "dead-URL path this whole item exists to avoid"), + (APPLY, "### Step 6b: Record the Application", + "`/outcome` Step 1.4", + "the derivation is no longer pinned to /outcome's, so a later edit to " + "either can silently orphan the archive"), + (OUTCOME, "## Step 1: Load State and Identify the Application", + "4. Derive the archive folder name", + "apply.md item 7 defers its folder derivation to /outcome Step 1.4 by " + "number; renumbering Step 1 leaves that citation dangling"), + (APPLY, "### Step 6b: Record the Application", + "**If the file already exists, leave it**", + "re-running /apply to refresh a CV would overwrite the posting that " + "was actually applied against"), + (APPLY, "### Step 6b: Record the Application", + "keeps the older posting", + "the leave-it rule would read as if the folder is always fresh, hiding " + "that a re-application to the same role collides with the old archive"), + (APPLY, "### Step 6b: Record the Application", + "left in place rather than written", + "the skip discards the current posting silently, and /interview preps " + "against the earlier application's posting"), + (APPLY, "### Step 6b: Record the Application", + "never reconstruct it from memory", + "a model that reached Step 6b without the text could satisfy none of " + "item 7's constraints, and would write a remembered posting instead"), + (SKILL, "### Step 1: Research & Evaluate Fit", + "full posting text verbatim", + "the /scrape path never runs /apply Step 0, so nothing stops it " + "compressing the posting before Step 3b archives it"), + (SKILL, "### Step 3b: Record the Application", + "same posting archive", + "the /scrape path reaches Step 3b without running /apply, and its " + "closed enumeration of Step 6b's rules would omit the archive write"), + (OUTCOME, "## Step 3: Archive the Application Materials", + "if it already exists, leave it", + "/outcome would overwrite /apply's archived posting with a re-fetch, " + "the dead-URL branch the /apply write exists to avoid"), + ] + + def test_posting_is_archived_where_every_reader_looks(self): + for path, heading, needle, why in self.CASES: + with self.subTest(file=path.name, rule=needle): + self.assertIn(needle, section(path, heading), why) + + +class DeadlineSurvivesEveryWrite(unittest.TestCase): + """#319: the deadline is carried through the whole pipeline and never dropped. + + The header migration must be header-line-only (inserting it mid-column + shifts every value of every existing row), and every path that rewrites + a tracker row (/outcome Step 4, /gmail-sync Step 7a) must preserve + fields it does not parse - the deadline is the first such field. + """ + + CASES = [ + (APPLY, "### Step 6b: Record the Application", "append `,deadline` to the header line only", + "a mid-header insert shifts every existing row's values by one position"), + (OUTCOME, "## Step 1: Load State and Identify the Application", + "append `,deadline` to the header line only", + "the two commands must migrate identically, or whichever runs first sets the schema"), + (APPLY, "## Step 0: Parse Input", "application deadline", + "Step 6b's value is supposed to come from Step 0's extraction, so the extraction " + "must be stated where the posting text is still held in full"), + (APPLY, "### Step 6b: Record the Application", "Never guess one", + "the deadline must stay empty when the posting states none - a guessed date is " + "the urgency clock firing on a date nobody set"), + (APPLY, "### Step 6b: Record the Application", "leave an existing deadline alone", + "absence is not a correction: a run that extracted no deadline must not blank " + "the one /apply already wrote"), + (OUTCOME, "## Step 1: Load State and Identify the Application", "Deadline urgency", + "a drafted row has nothing applied so the quiet clock must not run on it - the " + "deadline is the only clock that applies, and it must not be omitted"), + (OUTCOME, "## Step 1: Load State and Identify the Application", "never chased", + "surfacing the deadline must not drag drafted rows into the follow-up offer"), + (OUTCOME, "## Step 4: Update the Tracker", "preserve every other field of the row", + "a status update that rewrites the row would blank the deadline column"), + (GMAIL_SYNC, "### Step 7a: Write Approved Updates", "preserve every other field", + "the sync path rewrites the row too - it must carry the same preservation rule"), + (NOTION_SYNC, None, "**Deadline precedence: the tracker wins too**", + "the tracker's deadline (written from the posting the application was actually " + "built on) must override the scraper's stored value"), + (NOTION_SYNC, None, "tracker `deadline` column", + "the Deadine property must name the tracker column as its source"), + (SKILL, "### Step 3b: Record the Application", "`deadline` is the application deadline", + "the /scrape path reaches Step 3b without running /apply Step 0, so it must " + "still be told what the field is and where it comes from"), + # The two properties the migration has to hold. Both are stated in the + # prose of either file and neither was pinned, so either could be edited + # away with a green suite - turning an agreed header-line append into a + # row rewrite, which is a different and far riskier change. + (APPLY, "### Step 6b: Record the Application", "no data row is touched", + "a migration that rewrites rows is a different and far riskier change than " + "one that appends to the header line, and only the second was agreed"), + (OUTCOME, "## Step 1: Load State and Identify the Application", "no data row is touched", + "same rule, stated in both files, because either command may be the one that " + "meets a legacy tracker first"), + (APPLY, "### Step 6b: Record the Application", "read as an empty deadline", + "rows written before the migration have no fourteenth field; if that is not " + "stated, a reader may treat the short row as malformed and drop it"), + (OUTCOME, "## Step 1: Load State and Identify the Application", + "read as an empty deadline", + "same rule, stated in both files"), + (OUTCOME, "## Step 1: Load State and Identify the Application", + "one edit to an existing tracker", + "Step 4 forbids restructuring the CSV, so without this the header append reads " + "as a violation of the same command's own rule and an implementer has a " + "documented reason to skip the migration"), + (NOTION_SYNC, None, "never reconcile the two by picking the earlier or later date", + "the tracker-wins rule says which source to prefer but does not forbid the " + "plausible-looking min() of the two, which syncs a date the user never " + "applied against"), + ] + + def test_deadline_survives_every_write(self): + for path, heading, needle, why in self.CASES: + with self.subTest(file=path.name, rule=needle): + haystack = section(path, heading) if heading else path.read_text(encoding="utf-8") + self.assertIn(needle, haystack, why) + + +class ArchiveNameIsOnePathComponent(unittest.TestCase): + """`_` must derive a single path component. + + `Novo Nordisk A/S` used to derive `novo_nordisk_a/s_/`: every + command that *derives* the path agrees and keeps working, while the + two that *enumerate* `documents/applications/*/` (/setup Path A, + /html-report's glob) silently skip the nested archive. The character + rule lives in one place - documents/README.md's Subfolder naming + block - and the derivation sites cite it rather than restating it + (jakob1379/ai-job-search#22). + """ + + CASES = [ + (DOCS_README, "## applications/", + "not a letter, digit or underscore is dropped", + "the character rule is stated nowhere else; without it the naming " + "convention leaves `/` untouched and the archive nests"), + (DOCS_README, "## applications/", + "single path component", + "the sentence that says why the rule exists; without it the next " + "edit simplifies the rule back to spaces-only"), + (OUTCOME, "## Step 1: Load State and Identify the Application", + "by the **Subfolder naming** rule in `documents/README.md`", + "Step 1.4 is the derivation every other writer cites; paraphrasing " + "the rule here is how the two copies drifted apart originally"), + (APPLY, "### Requirement coverage (both documents)", + "the same rule `/outcome` Step 1.4 uses", + "CV and cover-letter filenames use the same unsanitised values; a " + "`/` there sends the draft to a path lualatex never writes a PDF " + "back to, and the Step 4 compile check fails on a phantom path"), + (SKILL, "### Step 2: Tailor CV", + "by the **Subfolder naming** rule in `documents/README.md`", + "the /scrape path writes its documents before Step 3b consults /apply, " + "so /apply's filename rule cannot protect it"), + (GMAIL_SYNC, "## Step 2: Load State", + "by the **Subfolder naming** rule in `documents/README.md`", + "gmail-sync both locates and creates archives; its old spaces-only " + "paraphrase would split state across two folders"), + (INTERVIEW, "## Step 1: Load the Application Context", + "by the **Subfolder naming** rule in `documents/README.md`", + "interview must read the same archive /apply and /outcome wrote"), + (INTERVIEW, "### 6. Logistics", + "archive folder derived in Step 1", + "interview must reuse its canonical read path when writing the prep pack"), + (NOTION_SYNC, "## Step 5: Write the Detail Page", + "by the **Subfolder naming** rule in `documents/README.md`", + "notion-sync otherwise reports that the sanitized local archive is absent"), + (DOCS_README, "## applications/", + "If the derived name is empty", + "dropping untrusted punctuation can produce no component at all, which " + "would write files directly under documents/applications"), + ] + + def test_the_rule_has_one_home_and_every_deriver_cites_it(self): + for path, heading, needle, why in self.CASES: + with self.subTest(file=path.name, rule=needle): + self.assertIn(needle, section(path, heading), why) + + @staticmethod + def derive(company, role): + """The Subfolder naming rule, executed exactly as documented: + lowercase, underscores for spaces, drop every character that is + not a letter/digit/underscore, collapse runs, trim the ends. + (\\w is Unicode in Python 3, so Danish letters survive.)""" + name = f"{company}_{role}".lower().replace(" ", "_") + name = re.sub(r"[^\w]", "", name) + name = re.sub(r"_+", "_", name).strip("_") + return name or None + + DERIVATIONS = [ + ("Novo Nordisk A/S", "Data Scientist", "novo_nordisk_as_data_scientist"), + ("Acme", "Data Scientist / ML Engineer", "acme_data_scientist_ml_engineer"), + ("Ørsted A/S", "ML Engineer", "ørsted_as_ml_engineer"), + # company/role reach the derivation from untrusted posting text + # (apply.md Step 0), so `..` must not survive either + ("../..", "Data Scientist", "data_scientist"), + ("../..", "///", None), + ] + + def test_documented_rule_yields_a_single_path_component(self): + for company, role, expected in self.DERIVATIONS: + with self.subTest(company=company, role=role): + name = self.derive(company, role) + self.assertEqual(name, expected) + if name is None: + continue + self.assertNotIn("/", name) + self.assertNotIn("..", name) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_check_framework_version.py b/tests/test_check_framework_version.py new file mode 100755 index 0000000..9562233 --- /dev/null +++ b/tests/test_check_framework_version.py @@ -0,0 +1,110 @@ +"""Guards for tools/check_framework_version.py - the CI gate itself. + +This gate is what stops a PR from editing a profile-bearing framework +file without bumping `framework_version` (the fork-rebase safety marker). +It ran in CI with zero tests, so a one-line mutation +(`return meaningful_changes > 0` -> `return False`) disabled it while +the whole suite stayed green (review finding F22, 2026-08-19). A broken +guard is silent by construction: nothing fails, it just stops catching. + +Each test builds an isolated git repo with the script copied inside it +(the script resolves ROOT from __file__), so the real repo is never read +or written. +""" +import os +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT = REPO_ROOT / "tools" / "check_framework_version.py" + +FRONTMATTER = "---\nframework_version: 1.0.0\n---\n" +BODY = "# Test framework file\n\nOriginal guidance sentence.\n" + + +class CheckerRepoFixture(unittest.TestCase): + def setUp(self): + self.root = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.root, ignore_errors=True) + + tools = self.root / "tools" + tools.mkdir() + shutil.copy(SCRIPT, tools / "check_framework_version.py") + + self.skill_dir = self.root / ".claude" / "skills" / "job-application-assistant" + self.skill_dir.mkdir(parents=True) + self.framework_file = self.skill_dir / "01-test-profile.md" + self.framework_file.write_text(FRONTMATTER + BODY, encoding="utf-8") + + self.git("init", "-q") + self.git("add", "-A") + self.git("commit", "-q", "-m", "base") + + def git(self, *args): + subprocess.run( + ["git", "-c", "user.name=test", "-c", "user.email=test@example.com", *args], + cwd=self.root, + check=True, + capture_output=True, + text=True, + ) + + def run_checker(self): + # Strip GitHub Actions variables so get_base_commit() takes the + # local path (uncommitted changes vs HEAD) regardless of where the + # test suite itself runs. + env = {k: v for k, v in os.environ.items() if not k.startswith("GITHUB_")} + return subprocess.run( + [sys.executable, str(self.root / "tools" / "check_framework_version.py")], + capture_output=True, + text=True, + env=env, + ) + + +class FrameworkVersionGateTests(CheckerRepoFixture): + def test_clean_tree_passes(self): + result = self.run_checker() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Framework Version Check: OK", result.stdout) + + def test_unbumped_edit_fails(self): + self.framework_file.write_text( + FRONTMATTER + BODY + "\nA new sentence without a version bump.\n", + encoding="utf-8", + ) + + result = self.run_checker() + + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("modified without bumping 'framework_version'", result.stdout) + + def test_bumped_edit_passes(self): + bumped = FRONTMATTER.replace("1.0.0", "1.0.1") + self.framework_file.write_text( + bumped + BODY + "\nA new sentence with a version bump.\n", + encoding="utf-8", + ) + + result = self.run_checker() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_file_without_version_marker_fails(self): + (self.skill_dir / "02-unmarked.md").write_text( + "# No frontmatter at all\n", encoding="utf-8" + ) + + result = self.run_checker() + + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("missing 'framework_version'", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_check_upstream_updates.py b/tests/test_check_upstream_updates.py new file mode 100755 index 0000000..9c92e06 --- /dev/null +++ b/tests/test_check_upstream_updates.py @@ -0,0 +1,164 @@ +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parent.parent +SCRIPT = REPO_ROOT / "tools" / "check_upstream_updates.py" + +TEMPLATE_URL = "https://github.com/MadsLorentzen/ai-job-search.git" +FORK_URL = "https://github.com/octocat/ai-job-search.git" + +FRAMEWORK_FILES = [ + ".claude/skills/job-application-assistant/01-candidate-profile.md", + ".claude/skills/job-application-assistant/02-behavioral-profile.md", + ".claude/skills/job-application-assistant/03-writing-style.md", + ".claude/skills/job-application-assistant/04-job-evaluation.md", + ".claude/skills/job-application-assistant/05-cv-templates.md", + ".claude/skills/job-application-assistant/06-cover-letter-templates.md", + ".claude/skills/job-application-assistant/07-interview-prep.md", + ".claude/skills/job-application-assistant/08-application-forms.md", + ".claude/skills/job-application-assistant/09-web-research.md", + ".claude/skills/job-application-assistant/SKILL.md", + "AGENTS.md", +] + +FRONTMATTER = "---\nframework_version: 1.0.0\n---\n" + + +class UpstreamCheckerRepoFixture(unittest.TestCase): + def setUp(self): + self.root = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.root, ignore_errors=True) + + tools = self.root / "tools" + tools.mkdir() + shutil.copy(SCRIPT, tools / "check_upstream_updates.py") + + for rel in FRAMEWORK_FILES: + path = self.root / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(FRONTMATTER, encoding="utf-8") + + subprocess.run(["git", "init", "-b", "master"], cwd=self.root, check=True, capture_output=True) + subprocess.run(["git", "config", "user.name", "Test"], cwd=self.root, check=True, capture_output=True) + subprocess.run(["git", "config", "user.email", "test@example.com"], cwd=self.root, check=True, capture_output=True) + subprocess.run(["git", "add", "-A"], cwd=self.root, check=True, capture_output=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=self.root, check=True, capture_output=True) + + def add_remote(self, name: str, url: str) -> None: + subprocess.run(["git", "remote", "add", name, url], cwd=self.root, check=True, capture_output=True) + + def materialize_remote_ref(self, name: str) -> None: + subprocess.run( + ["git", "update-ref", f"refs/remotes/{name}/master", "HEAD"], + cwd=self.root, + check=True, + capture_output=True, + ) + + def run_checker(self, *args) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(self.root / "tools" / "check_upstream_updates.py"), "--no-fetch", *args], + cwd=self.root, + capture_output=True, + text=True, + ) + + +class ForkWithoutUpstreamRemoteTests(UpstreamCheckerRepoFixture): + def setUp(self): + super().setUp() + self.add_remote("origin", FORK_URL) + self.materialize_remote_ref("origin") + + def test_fork_fallback_warns_that_check_is_against_own_fork(self): + result = self.run_checker("--remote", "upstream") + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Falling back to 'origin'", result.stdout) + self.assertIn("does not point to the ai-job-search template repo", result.stdout) + self.assertNotIn("up to date with upstream!", result.stdout) + self.assertIn("up to date with origin/master", result.stdout) + + +class DirectCloneFallbackTests(UpstreamCheckerRepoFixture): + def setUp(self): + super().setUp() + self.add_remote("origin", TEMPLATE_URL) + self.materialize_remote_ref("origin") + + def test_clone_of_template_falls_back_without_fork_warning(self): + result = self.run_checker("--remote", "upstream") + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Falling back to 'origin'", result.stdout) + self.assertNotIn("does not point to the ai-job-search template repo", result.stdout) + self.assertIn("up to date with origin/master", result.stdout) + + def test_clone_with_lowercased_template_url_falls_back_without_fork_warning(self): + # GitHub serves repo paths case-insensitively, so a clone from + # https://github.com/madslorentzen/ai-job-search is still the template. + subprocess.run( + ["git", "remote", "set-url", "origin", TEMPLATE_URL.lower()], + cwd=self.root, + check=True, + capture_output=True, + ) + + result = self.run_checker("--remote", "upstream") + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Falling back to 'origin'", result.stdout) + self.assertNotIn("does not point to the ai-job-search template repo", result.stdout) + + +class UpstreamRemotePresentTests(UpstreamCheckerRepoFixture): + def setUp(self): + super().setUp() + self.add_remote("origin", FORK_URL) + self.add_remote("upstream", TEMPLATE_URL) + self.materialize_remote_ref("upstream") + + def test_explicit_upstream_remote_is_used_without_warning(self): + result = self.run_checker() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertNotIn("Falling back to 'origin'", result.stdout) + self.assertNotIn("does not point to the ai-job-search template repo", result.stdout) + self.assertIn("up to date with upstream/master", result.stdout) + + +class UpstreamRefMissingFileTests(UpstreamCheckerRepoFixture): + """Simulates upstream renaming/deleting one framework file while the + fork still has its own copy: git show then fails, and the checker used + to swallow the error and report a clean '[OK]'.""" + + def setUp(self): + super().setUp() + self.add_remote("origin", FORK_URL) + self.add_remote("upstream", TEMPLATE_URL) + + # Upstream drops AGENTS.md (rename/delete) in a new commit. + subprocess.run(["git", "rm", "-q", "AGENTS.md"], cwd=self.root, check=True, capture_output=True) + subprocess.run(["git", "commit", "-qm", "drop AGENTS.md"], cwd=self.root, check=True, capture_output=True) + self.materialize_remote_ref("upstream") + + # The fork keeps its own copy locally, so only the upstream side + # lacks the file. + (self.root / "AGENTS.md").write_text(FRONTMATTER, encoding="utf-8") + + def test_file_missing_upstream_is_reported_instead_of_silent_ok(self): + result = self.run_checker() + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("AGENTS.md", result.stdout) + self.assertNotIn("[OK] All framework files are up to date", result.stdout) + self.assertIn("[WARNING]", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_convert_salary_excel.py b/tests/test_convert_salary_excel.py new file mode 100755 index 0000000..665825e --- /dev/null +++ b/tests/test_convert_salary_excel.py @@ -0,0 +1,342 @@ +import unittest +from types import SimpleNamespace + +from tools.convert_salary_excel import ( + INDEX_PATTERNS, + detect_column_type, + header_matches, + parse_numeric_cell, + parse_sheet, +) + + +class FakeWorksheet: + title = "Sheet1" + + def __init__(self, rows): + self.rows = rows + + def iter_rows(self, min_row=1, max_row=None, values_only=False): + rows = self.rows[min_row - 1:max_row] + for row in rows: + if values_only: + yield row + else: + yield [SimpleNamespace(value=value) for value in row] + + def __getitem__(self, row_number): + return [SimpleNamespace(value=value) for value in self.rows[row_number - 1]] + + +class DetectColumnTypeTests(unittest.TestCase): + def test_index_headers_are_not_misclassified_as_count(self): + for header in ("Index", "Salary Index", "Engineering Index", "Median salary"): + with self.subTest(header=header): + self.assertEqual(detect_column_type(header), "index") + + def test_single_letter_n_only_matches_as_a_token(self): + self.assertEqual(detect_column_type("Employee n"), "count") + self.assertEqual(detect_column_type("Engineering"), None) + + def test_count_headers_still_match_common_labels(self): + for header in ("Count", "Engineering Count", "Antal medarbejdere"): + with self.subTest(header=header): + self.assertEqual(detect_column_type(header), "count") + + def test_count_inside_word_does_not_make_count_header(self): + self.assertIsNone(detect_column_type("Accounting Total")) + self.assertEqual(detect_column_type("Accounting Index"), "index") + + def test_danish_compound_headers_still_match(self): + self.assertEqual(detect_column_type("Lønindeks"), "index") + + def test_compound_patterns_match_as_substring_but_others_do_not(self): + # A compound token (Danish "løn") matches inside a glued header word. + self.assertTrue(header_matches("lønindeks", INDEX_PATTERNS)) + # A pattern that is not a compound token ("salary") only matches as a + # whole token, so it must not match inside an unrelated glued word. + self.assertFalse(header_matches("salaryindex", INDEX_PATTERNS)) + self.assertTrue(header_matches("salary index", INDEX_PATTERNS)) + + def test_parse_sheet_preserves_category_name_with_letter_n(self): + ws = FakeWorksheet([ + ("Company", "Engineering Count", "Engineering Index"), + ("Example Corp", 12, 105.5), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(companies[0]["categories"]["engineering"], {"count": 12, "index": 105.5}) + + def test_parse_sheet_groups_accounting_count_index_pair(self): + ws = FakeWorksheet([ + ("Company", "Accounting Count", "Accounting Index"), + ("Example Corp", 12, 105.5), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(companies[0]["categories"]["accounting"], {"count": 12, "index": 105.5}) + + def test_parse_sheet_normalizes_paired_category_name_with_underscores(self): + ws = FakeWorksheet([ + ("Company", "Software Engineering Count", "Software Engineering Index"), + ("Example Corp", 8, 110.0), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(companies[0]["categories"]["software_engineering"], {"count": 8, "index": 110.0}) + + def test_parse_sheet_detects_company_column_with_token_header(self): + # Real-world salary sheets rarely use the bare token "Company"; + # headers like "Company Name" / "Employer Name" must still be + # detected as the company column (previously silently skipped -> []). + for header in ("Company", "Company Name", "Employer Name"): + with self.subTest(header=header): + ws = FakeWorksheet([ + (header, "Salary"), + ("Example Corp", 105.5), + ]) + companies = parse_sheet(ws) + self.assertEqual(len(companies), 1) + self.assertEqual(companies[0]["company"], "Example Corp") + self.assertEqual( + companies[0]["categories"]["salary"], {"index": 105.5} + ) + + def test_parse_sheet_detects_city_column_with_token_header(self): + # City headers are matched with the same token-based header_matches() + # used for the company column, not exact string equality. Real-world + # sheets rarely use the bare token "City" or "Kommune" alone; headers + # like "City Name" / "City/Kommune" must still be detected as the city + # column (previously silently left as city_col=None -> empty city). + for header in ("City", "City Name", "Kommune", "City/Kommune"): + with self.subTest(header=header): + ws = FakeWorksheet([ + ("Company", header, "Salary"), + ("Example Corp", "Aarhus", 105.5), + ]) + companies = parse_sheet(ws) + self.assertEqual(len(companies), 1) + self.assertEqual(companies[0]["city"], "Aarhus") + + def test_parse_sheet_handles_ragged_rows(self): + # openpyxl's read_only mode yields ragged tuples for dimension-less + # workbooks: a row can be shorter than the header. A company row that + # omits its city and category cells must parse without an IndexError, + # be retained, and get an empty city. + ws = FakeWorksheet([ + ("Company", "City", "Engineering Count", "Engineering Index"), + ("Example Corp",), + ("Other Corp", "Aarhus", 12, 105.5), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(len(companies), 2) + self.assertEqual(companies[0]["company"], "Example Corp") + self.assertEqual(companies[0]["city"], "") + self.assertEqual(companies[0]["categories"], {}) + self.assertEqual(companies[1]["categories"]["engineering"], {"count": 12, "index": 105.5}) + + def test_parse_sheet_skips_row_shorter_than_company_column(self): + # A ragged row that ends before the company column has no company cell + # at all; it must be skipped, not crash the parse. + ws = FakeWorksheet([ + ("Notes", "Company", "Salary Index"), + ("stray",), + ("", "Example Corp", 105.5), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(len(companies), 1) + self.assertEqual(companies[0]["company"], "Example Corp") + + def test_skips_free_text_column(self): + # A free-text "Notes" column must not become a bogus salary category. + ws = FakeWorksheet([ + ("Company", "Salary Index", "Notes"), + ("Example Corp", 105.5, "good"), + ]) + + companies = parse_sheet(ws) + + self.assertIn("salary_index", companies[0]["categories"]) + self.assertNotIn("notes", companies[0]["categories"]) + + def test_skips_numeric_identifier_column(self): + # A numeric "Id" column (employee id) must not be treated as a salary index. + ws = FakeWorksheet([ + ("Company", "Salary Index", "Id"), + ("Example Corp", 105.5, 7), + ]) + + companies = parse_sheet(ws) + + self.assertIn("salary_index", companies[0]["categories"]) + self.assertNotIn("id", companies[0]["categories"]) + + def test_keeps_numeric_salary_column(self): + # A genuine numeric salary column still produces a salary category. + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", 105.5), + ]) + + companies = parse_sheet(ws) + + self.assertIn("salary_index", companies[0]["categories"]) + self.assertEqual(companies[0]["categories"]["salary_index"], {"index": 105.5}) + + def test_parse_sheet_accepts_comma_decimal_string_values(self): + # Locale-formatted Excel exports can carry numeric cells as strings. + # Danish decimal commas must not be silently dropped by float(). + ws = FakeWorksheet([ + ("Company", "Engineering Count", "Engineering Index"), + ("Example Corp", "12,0", "108,5"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["engineering"], + {"count": 12, "index": 108.5}, + ) + + def test_parse_sheet_accepts_danish_thousands_and_decimal_string(self): + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", "1.234,5"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["salary_index"], + {"index": 1234.5}, + ) + + def test_parse_sheet_skips_ambiguous_single_comma_thousands_string(self): + # In an English-locale export, "1,234" is probably 1234, but in a + # decimal-comma locale it could be 1.234. Preserve the old safe-skip + # behavior instead of guessing and writing a 1000x-wrong salary value. + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", "1,234"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(companies[0]["categories"], {}) + + def test_parse_sheet_skips_ambiguous_single_dot_thousands_string(self): + # "1.234" is the dot-side mirror of the comma guard above: in a + # decimal-dot locale it is 1.234, while a Danish export (whole + # thousands, no decimal comma, e.g. "60.000") means 1234/60000. + # float() used to write the 1000x-smaller value silently - the + # same never-guess policy must apply to both separators. + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", "1.234"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(companies[0]["categories"], {}) + + def test_parse_sheet_pairs_interleaved_count_index_columns_by_name(self): + ws = FakeWorksheet([ + ("Company", "Antal kvinder", "Antal mænd", "Kvinder indeks", "Mænd indeks"), + ("Example Corp", 15, 20, 95.0, 108.0), + ]) + + companies = parse_sheet(ws) + + categories = companies[0]["categories"] + self.assertEqual(categories["kvinder"], {"count": 15, "index": 95.0}) + self.assertEqual(categories["mænd"], {"count": 20, "index": 108.0}) + + def test_standalone_count_column_is_stored_as_count_not_index(self): + # A count column with no matching index column (e.g. a lone total + # headcount) is still count data. It must not be emitted as a salary + # index, which salary_lookup would render with a bogus "vs baseline" + # percentage. The paired category alongside it is unaffected. + ws = FakeWorksheet([ + ("Company", "Antal", "IT Count", "IT Index"), + ("Example Corp", 250, 30, 108.5), + ]) + + companies = parse_sheet(ws) + + categories = companies[0]["categories"] + self.assertEqual(categories["antal"], {"count": 250}) + self.assertEqual(categories["it"], {"count": 30, "index": 108.5}) + + def test_parse_sheet_non_adjacent_columns_no_cross_match(self): + ws = FakeWorksheet([ + ("Company", "Count_A", "Count_B", "Index_A", "Index_B"), + ("Example Corp", 10, 20, 100.0, 200.0), + ]) + + companies = parse_sheet(ws) + + categories = companies[0]["categories"] + self.assertEqual(categories["a"], {"count": 10, "index": 100.0}) + self.assertEqual(categories["b"], {"count": 20, "index": 200.0}) + + +if __name__ == "__main__": + unittest.main() + + +class ParseNumericCellLocaleTests(unittest.TestCase): + # The separator that appears LAST is the decimal separator. Assuming + # European ("." thousands, "," decimal) for every both-separator string + # turned a US "1,234.56" into 1.23456 - a silent 1000x corruption that + # flowed into salary_data.json and negotiation advice. + + def test_us_thousands_and_decimal_string(self): + self.assertEqual(parse_numeric_cell("1,234.56"), 1234.56) + + def test_us_multiple_thousands_groups(self): + self.assertEqual(parse_numeric_cell("1,234,567.89"), 1234567.89) + + def test_european_thousands_and_decimal_string(self): + self.assertEqual(parse_numeric_cell("1.234,56"), 1234.56) + + def test_european_multiple_thousands_groups(self): + self.assertEqual(parse_numeric_cell("1.234.567,89"), 1234567.89) + + +class CompoundCategoryPairingTests(unittest.TestCase): + def test_parse_sheet_pairs_danish_compound_index_with_count(self): + # "Lønindeks alle" is *detected* as an index column via + # COMPOUND_PATTERNS, but the derived category name must also lose the + # compound word or it can never pair with "Antal alle" ("alle" vs + # "lønindeks alle") - exactly the locale the compound support exists for. + ws = FakeWorksheet([ + ("Firma", "Antal alle", "Lønindeks alle"), + ("Example Corp", 12, 118.0), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["alle"], + {"count": 12, "index": 118.0}, + ) + + def test_parse_sheet_sheet_level_us_locale_value(self): + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", "1,234.56"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["salary_index"], + {"index": 1234.56}, + ) diff --git a/tests/test_gmail_sync_command.py b/tests/test_gmail_sync_command.py new file mode 100755 index 0000000..deb9b49 --- /dev/null +++ b/tests/test_gmail_sync_command.py @@ -0,0 +1,43 @@ +"""Guards for /gmail-sync's Gmail query semantics. + +The command's stated intent is "skip sent/drafts - status signals come +from what employers send you". `in:inbox` does not mean that: it matches +only messages currently IN the inbox, so it also excludes every archived +message - and, self-defeatingly, the mail matched by the very +job-search label Step 3.1 hunts for, because the standard filter that +applies such a label also archives ("skip the inbox"). The correct +operators for the stated intent are `-in:sent -in:drafts` (review +finding F18, 2026-08-19). The failure mode is silent under-detection: a +missed rejection or interview invite just looks like "no updates". +""" +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +GMAIL_SYNC = REPO / ".claude" / "commands" / "gmail-sync.md" + + +class TestGmailQueryOperators(unittest.TestCase): + def setUp(self): + self.text = GMAIL_SYNC.read_text(encoding="utf-8") + + def test_query_excludes_sent_and_drafts_explicitly(self): + self.assertIn( + "-in:sent -in:drafts", + self.text, + "the query must exclude sent/drafts with negative operators, " + "which keep archived and label-filtered mail in scope", + ) + + def test_query_never_restricts_to_the_inbox(self): + self.assertNotIn( + "in:inbox", + self.text.replace("-in:sent", "").replace("-in:drafts", ""), + "in:inbox silently drops archived mail and everything a " + "label-and-archive filter routed past the inbox - exactly the " + "mail the label search in Step 3.1 exists to find", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_html_report_command.py b/tests/test_html_report_command.py new file mode 100755 index 0000000..5e8900c --- /dev/null +++ b/tests/test_html_report_command.py @@ -0,0 +1,165 @@ +"""Tests for the /html-report command and its gitignore rule. + +Mirrors the pattern in test_security_guards.py: one class that verifies +properties of the real repo, testing the things CI would catch if the +command file or gitignore rule were wrong. +""" + +import re +import subprocess +import sys +import unittest +from pathlib import Path + +try: + import yaml # noqa: F401 - only probing availability for the lint integration test + _HAVE_YAML = True +except ImportError: + _HAVE_YAML = False + +REPO_ROOT = Path(__file__).resolve().parent.parent +COMMAND_FILE = REPO_ROOT / ".claude" / "commands" / "html-report.md" +LINT_SCRIPT = REPO_ROOT / "tools" / "lint_skills.py" +GITIGNORE = REPO_ROOT / ".gitignore" + + +class HtmlReportCommandFileTests(unittest.TestCase): + """Structural checks on the command file itself.""" + + def test_command_file_exists(self): + self.assertTrue(COMMAND_FILE.exists(), f"{COMMAND_FILE} not found") + + def test_command_file_starts_with_correct_header(self): + """lint_skills.py rejects command files that don't start with '# /'.""" + text = COMMAND_FILE.read_text(encoding="utf-8") + first_line = text.lstrip().splitlines()[0] + self.assertTrue( + first_line.startswith("# /html-report"), + f"Command file must start with '# /html-report', got: {first_line!r}", + ) + + def test_command_file_is_non_empty(self): + text = COMMAND_FILE.read_text(encoding="utf-8").strip() + self.assertGreater(len(text), 100, "Command file appears suspiciously short") + + +class HtmlReportTrackerFieldTests(unittest.TestCase): + """The dashboard is a consumer of every tracker column: the Step 1 field + enumeration and the Step 3 table columns must stay in phase with the + canonical 14-column header (apply.md /outcome.md Step 1.1), so a future + column addition cannot silently vanish from the dashboard the way + `deadline` did.""" + + # Derived, never copied: a header literal repeated in this file drifts in + # lockstep with the spec it polices - add a 15th column to apply.md and a + # stale hardcoded 14-column list still passes every comparison here (a + # 14-column string is a substring of a 15-column header). Reading the + # canonical line back from apply.md makes the simulated drift fail with a + # clean list diff naming the missing column instead. + CANONICAL_HEADER = re.search( + r"^\s*(date,company,[a-z_,]+)$", + (REPO_ROOT / ".claude" / "commands" / "apply.md").read_text(encoding="utf-8"), + re.M, + ).group(1).split(",") + + def test_step1_parses_every_canonical_tracker_column(self): + text = COMMAND_FILE.read_text(encoding="utf-8") + match = re.search( + r"Parse every row into a record with fields:\n\s+((?:`[^`]+`,?\s*)+)", + text, + ) + self.assertIsNotNone(match, "Step 1 field enumeration not found") + fields = [f.strip() for f in re.findall(r"`([^`]+)`", match.group(1))] + self.assertEqual(fields, self.CANONICAL_HEADER) + + def test_step3_table_columns_include_deadline_after_date(self): + """Date · Deadline order is the whole point of the change: the dashboard + must surface the clock that drives `/rank`'s urgency next to the date. + A membership pair (both `Date` and `Deadline` present somewhere) cannot + tell a swapped order from the correct one, and the order is what the + table shows the reader.""" + text = COMMAND_FILE.read_text(encoding="utf-8") + match = re.search(r"### Table: columns to include\n\n(.+)\n", text) + self.assertIsNotNone(match, "Step 3 table column list not found") + line = match.group(1) + self.assertIn( + "`Date` · `Deadline` · `Company`", + line, + "Step 3 must offer the Deadline column directly after Date - the " + "list defines the dashboard's column order, and a swapped order " + "reads as a different table", + ) + + +class HtmlReportGitignoreTests(unittest.TestCase): + """reports/ must be gitignored — it holds personal generated output.""" + + def test_funnel_is_computed_from_stage_history_not_current_status(self): + """status is a current state, not a history: an application that + interviewed and was then rejected carries status `rejected` and would + never count as having reached Interview, so a finished search reads + as though nobody ever interviewed (review finding F10, 2026-08-19). + The stage checkboxes merged from outcome.md in Step 1.2 are the + history; the funnel must be told to use them.""" + text = COMMAND_FILE.read_text(encoding="utf-8") + self.assertIn( + "stage checkboxes", + text.split("## Step 3")[0].split("## Step 2")[1], + "Step 2's funnel definition must derive stage-reached from the " + "merged outcome.md stage checkboxes", + ) + self.assertIn( + "not current status", + text, + "the funnel rule must say explicitly that current status alone undercounts", + ) + + def test_rejection_rate_excludes_declined_offers_and_withdrawals(self): + """offer_declined is the candidate turning an offer down (a success) + and withdrawn is candidate-initiated; counting either as a rejection + inflates the rejection rate on a self-assessment dashboard (review + finding F11, 2026-08-19).""" + text = COMMAND_FILE.read_text(encoding="utf-8") + self.assertIn( + "`offer_declined`", + text.split("## Step 3")[0].split("## Step 2")[1], + "the rejection-rate definition must address offer_declined", + ) + self.assertIn( + "not rejections", + text, + "the rate must exclude candidate-initiated outcomes explicitly", + ) + + def test_reports_folder_is_gitignored(self): + rules = {line.strip() for line in GITIGNORE.read_text(encoding="utf-8").splitlines()} + self.assertIn( + "reports/", + rules, + "reports/ must be listed in .gitignore — generated dashboards are personal output", + ) + + +@unittest.skipUnless( + _HAVE_YAML, + "PyYAML not installed (the CI Python-test job omits it; the lint job runs lint_skills.py directly)", +) +class HtmlReportLintIntegrationTests(unittest.TestCase): + """lint_skills.py must pass after the command is added.""" + + def test_lint_passes_on_real_repo(self): + result = subprocess.run( + [sys.executable, str(LINT_SCRIPT)], + capture_output=True, + text=True, + ) + self.assertEqual( + result.returncode, + 0, + f"lint_skills.py failed:\n{result.stdout}{result.stderr}", + ) + self.assertIn("OK", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_latex_guidance.py b/tests/test_latex_guidance.py new file mode 100755 index 0000000..ca8539a --- /dev/null +++ b/tests/test_latex_guidance.py @@ -0,0 +1,140 @@ +"""Guards for the LaTeX authoring guidance and the example documents. + +Three silent-failure modes live here, all found by the 2026-08-19 review +(F9, F31, F34). Each one produces a clean compile and a green CI run +while the rendered document or its ATS extraction is wrong, so the spec +files and the example sources are the only place a test can catch them: + +- F9: a bullet written as `\\item [text]` is parsed as moderncv's + optional label, rendered off the left page edge, and dropped from the + PDF text layer. The example CV shipped that way for months. +- F31: an unescaped `%` in body text silently truncates the rest of the + line (`&` at least fails loudly). The guidance must name the escapes. +- F34: `pdftotext` without `-enc UTF-8` emits Latin-1 on Xpdf builds, + so a correct Danish CV fails the documented "no replacement + characters" check and the agent is sent to "fix" a healthy document. +""" +import re +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +SKILL_DIR = REPO / ".claude" / "skills" / "job-application-assistant" +CV_TEMPLATES = SKILL_DIR / "05-cv-templates.md" +COVER_TEMPLATES = SKILL_DIR / "06-cover-letter-templates.md" +APPLY = REPO / ".claude" / "commands" / "apply.md" +EXAMPLE_CV = REPO / "cv" / "main_example.tex" +EXAMPLE_COVER = REPO / "cover_letters" / "cover_example.tex" + +# \item whose body starts with [ - with or without whitespace between. +# LaTeX skips spaces while scanning for the optional argument, so +# `\item [text]` and `\item[text]` both swallow the text as a label. +# The safe spelling `\item {[text]}` does not match. +UNBRACED_BRACKET_ITEM = re.compile(r"\\item\s*\[") + +# The escapes both guidance files must document. `%` is the load-bearing +# one: it truncates silently. The others fail loudly or corrupt spacing. +REQUIRED_ESCAPES = ["\\&", "\\%", "\\$", "\\#", "\\_"] + + +def section(text, heading): + """Return the body of a markdown section up to the next heading.""" + pattern = re.compile( + rf"^#+ {re.escape(heading)}[^\n]*\n(.*?)(?=^#+ |\Z)", + re.MULTILINE | re.DOTALL, + ) + match = pattern.search(text) + return match.group(1) if match else None + + +class TestBulletBracketTrap(unittest.TestCase): + """F9: no document or template doc may teach `\\item [text]`.""" + + def assert_no_unbraced_bracket_items(self, path): + offending = [ + f"{path.name}:{lineno}: {line.strip()}" + for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1) + if UNBRACED_BRACKET_ITEM.search(line) + ] + self.assertEqual( + offending, + [], + "\\item followed by [ is parsed as an optional label and the " + "text is clipped off the page; write \\item {[...]} instead:\n" + + "\n".join(offending), + ) + + def test_example_cv_has_no_bracket_labelled_bullets(self): + self.assert_no_unbraced_bracket_items(EXAMPLE_CV) + + def test_example_cover_letter_has_no_bracket_labelled_bullets(self): + self.assert_no_unbraced_bracket_items(EXAMPLE_COVER) + + def test_cover_letter_guide_does_not_teach_the_broken_pattern(self): + self.assert_no_unbraced_bracket_items(COVER_TEMPLATES) + + def test_cv_guide_does_not_teach_the_broken_pattern(self): + self.assert_no_unbraced_bracket_items(CV_TEMPLATES) + + +class TestSpecialCharacterGuidance(unittest.TestCase): + """F31: both template guides must document the LaTeX escapes.""" + + def assert_escapes_documented(self, path): + body = section(path.read_text(encoding="utf-8"), "LaTeX Special Characters") + self.assertIsNotNone( + body, f"{path.name} has no 'LaTeX Special Characters' section" + ) + missing = [esc for esc in REQUIRED_ESCAPES if esc not in body] + self.assertEqual( + missing, + [], + f"{path.name}'s special-characters section is missing: {missing}", + ) + + def test_cv_guide_documents_the_escapes(self): + self.assert_escapes_documented(CV_TEMPLATES) + + def test_cover_letter_guide_documents_the_escapes(self): + self.assert_escapes_documented(COVER_TEMPLATES) + + def test_cv_guide_warns_that_percent_truncates_silently(self): + body = section( + CV_TEMPLATES.read_text(encoding="utf-8"), "LaTeX Special Characters" + ) + self.assertIsNotNone(body) + self.assertRegex( + body, + re.compile(r"silent", re.IGNORECASE), + "the % failure mode must be called out as silent - it is the " + "reason this section exists (a clean compile with the rest of " + "the bullet gone)", + ) + + +class TestAtsExtractionEncoding(unittest.TestCase): + """F34: every documented extraction command must pin the encoding.""" + + def assert_pdftotext_commands_pin_utf8(self, path): + offending = [ + f"{path.name}:{lineno}: {line.strip()}" + for lineno, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1) + if "pdftotext" in line and "-layout" in line and "-enc UTF-8" not in line + ] + self.assertEqual( + offending, + [], + "pdftotext without -enc UTF-8 emits Latin-1 on Xpdf builds, so " + "the ATS check reports phantom replacement characters on any " + "non-ASCII CV; add -enc UTF-8:\n" + "\n".join(offending), + ) + + def test_apply_extraction_command_pins_utf8(self): + self.assert_pdftotext_commands_pin_utf8(APPLY) + + def test_cv_guide_extraction_command_pins_utf8(self): + self.assert_pdftotext_commands_pin_utf8(CV_TEMPLATES) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_lint_skills.py b/tests/test_lint_skills.py new file mode 100755 index 0000000..a7f57b3 --- /dev/null +++ b/tests/test_lint_skills.py @@ -0,0 +1,173 @@ +import json +import shutil +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parent.parent +LINTER_SCRIPT = REPO_ROOT / "tools" / "lint_skills.py" + + +def run_linter(root: Path) -> subprocess.CompletedProcess: + return subprocess.run( + [sys.executable, str(root / "tools" / "lint_skills.py")], + capture_output=True, + text=True, + ) + + +class LinterRepoFixture(unittest.TestCase): + def setUp(self): + self.root = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.root, ignore_errors=True) + + tools = self.root / "tools" + tools.mkdir() + shutil.copy(LINTER_SCRIPT, tools / "lint_skills.py") + # The Python-test CI job does not install PyYAML; the separate lint job + # does. These settings-focused tests only need a valid frontmatter map. + # The stub parses simple "key: value" lines, enough for the flat + # frontmatter these fixtures write, so the checks under test see the + # actual file content instead of a canned mapping. + (tools / "yaml.py").write_text( + "class YAMLError(Exception):\n" + " pass\n\n" + "def safe_load(text):\n" + " result = {}\n" + " for line in (text or '').splitlines():\n" + " if ':' in line:\n" + " key, _, value = line.partition(':')\n" + " result[key.strip()] = value.strip()\n" + " return result\n", + encoding="utf-8", + ) + + command = self.root / ".claude" / "commands" / "setup.md" + command.parent.mkdir(parents=True) + command.write_text("# /setup - Test setup command\n", encoding="utf-8") + + skill = self.root / ".claude" / "skills" / "example" / "SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text( + "---\nname: example\ndescription: Example skill\n---\n", + encoding="utf-8", + ) + + self.settings = self.root / ".claude" / "settings.json" + self.write_settings({"permissions": {"allow": []}}) + + def write_settings(self, data): + self.settings.write_text(json.dumps(data), encoding="utf-8") + + +class SettingsShapeTests(LinterRepoFixture): + def test_valid_settings_pass(self): + result = run_linter(self.root) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("lint_skills: OK", result.stdout) + + def test_invalid_json_fails_cleanly(self): + self.settings.write_text("{not json", encoding="utf-8") + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn(".claude/settings.json", result.stdout) + self.assertNotIn("Traceback", result.stderr) + + def test_non_object_root_fails_cleanly(self): + for data in ([], "settings", 1, None): + with self.subTest(data=data): + self.write_settings(data) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("top-level JSON value to be an object", result.stdout) + self.assertNotIn("Traceback", result.stderr) + + def test_non_object_permissions_fails_cleanly(self): + for permissions in ([], "permissions", 1, None): + with self.subTest(permissions=permissions): + self.write_settings({"permissions": permissions}) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("expected permissions to be an object", result.stdout) + self.assertNotIn("Traceback", result.stderr) + + def test_non_list_allow_fails_cleanly(self): + for allow in ({}, "Bash(bun run:*)", 1, None): + with self.subTest(allow=allow): + self.write_settings({"permissions": {"allow": allow}}) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("expected permissions.allow to be a list", result.stdout) + self.assertNotIn("Traceback", result.stderr) +class SkillAndCommandCheckTests(LinterRepoFixture): + """check_skill()/check_command() are the linter's main job and were + previously untested - only check_settings() had coverage, so deleting + e.g. the missing-allowed-tools error survived the whole suite (review + finding F23, 2026-08-19).""" + + def write_skill(self, frontmatter: str): + skill = self.root / ".claude" / "skills" / "example" / "SKILL.md" + skill.write_text(frontmatter, encoding="utf-8") + + def test_allowed_tools_referencing_a_missing_file_fails(self): + self.write_skill( + "---\n" + "name: example\n" + "description: Example skill\n" + "allowed-tools: Bash(bun run .claude/skills/example/DOES_NOT_EXIST.ts *)\n" + "---\n" + ) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("allowed-tools references a missing file", result.stdout) + self.assertIn("DOES_NOT_EXIST.ts", result.stdout) + + def test_allowed_tools_referencing_an_existing_file_passes(self): + target = self.root / ".claude" / "skills" / "example" / "cli.ts" + target.write_text("// present\n", encoding="utf-8") + self.write_skill( + "---\n" + "name: example\n" + "description: Example skill\n" + "allowed-tools: Bash(bun run .claude/skills/example/cli.ts *)\n" + "---\n" + ) + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_frontmatter_missing_description_fails(self): + self.write_skill("---\nname: example\ndescription:\n---\n") + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("missing required key 'description'", result.stdout) + + def test_command_without_slash_title_fails(self): + command = self.root / ".claude" / "commands" / "setup.md" + command.write_text("# setup - missing the slash\n", encoding="utf-8") + + result = run_linter(self.root) + + self.assertEqual(result.returncode, 1) + self.assertIn("must start with a '# /' title", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_notion_sync_command.py b/tests/test_notion_sync_command.py new file mode 100755 index 0000000..0f48892 --- /dev/null +++ b/tests/test_notion_sync_command.py @@ -0,0 +1,69 @@ +"""Guards for the /notion-sync command spec. + +The command is a markdown spec (the spec IS the implementation), so these +tests pin the invariants that would break silently: the header format that +lint_skills.py enforces, the gitignore entry that keeps the personal sync +state out of version control, and the privacy rule that forbids syncing +document content to Notion. +""" +import subprocess +import sys +import unittest +from pathlib import Path + +try: + import yaml # noqa: F401 - only probing availability for the lint integration test + _HAVE_YAML = True +except ImportError: + _HAVE_YAML = False + +REPO = Path(__file__).resolve().parent.parent +COMMAND = REPO / ".claude" / "commands" / "notion-sync.md" +GITIGNORE = REPO / ".gitignore" + + +class NotionSyncCommandSpec(unittest.TestCase): + def test_command_file_exists_with_lint_compliant_header(self): + self.assertTrue(COMMAND.is_file(), "command spec missing") + first_line = COMMAND.read_text(encoding="utf-8").splitlines()[0] + self.assertTrue( + first_line.startswith("# /notion-sync"), + f"header must start with '# /notion-sync' (lint_skills.py enforces it), got: {first_line!r}", + ) + + def test_command_file_is_substantive(self): + text = COMMAND.read_text(encoding="utf-8") + for section in ("## Step 0", "## Step 4", "## Important Rules"): + self.assertIn(section, text, f"spec lost its {section!r} section") + + def test_personal_sync_state_is_gitignored(self): + self.assertIn( + "job_scraper/notion_sync.json", + GITIGNORE.read_text(encoding="utf-8"), + "notion_sync.json is personal state and must never be committable", + ) + + def test_privacy_rule_documents_never_sync(self): + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "never upload, attach, or embed", + text, + "spec lost the rule that CV/cover-letter content never syncs to Notion", + ) + + @unittest.skipUnless( + _HAVE_YAML, + "PyYAML not installed (the CI Python-test job omits it; the lint job runs lint_skills.py directly)", + ) + def test_lint_skills_passes(self): + result = subprocess.run( + [sys.executable, str(REPO / "tools" / "lint_skills.py")], + cwd=REPO, + capture_output=True, + text=True, + ) + self.assertEqual(result.returncode, 0, f"lint_skills.py failed:\n{result.stdout}{result.stderr}") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_onboarding_privacy.py b/tests/test_onboarding_privacy.py new file mode 100755 index 0000000..8456022 --- /dev/null +++ b/tests/test_onboarding_privacy.py @@ -0,0 +1,85 @@ +"""Guards for the onboarding privacy warnings (issue #345). + +The README's quick start walks a new user into creating a public fork +(forks of public repos cannot be private) and then has /setup write +personal data into tracked files, with the only complete warning sitting +in SETUP.md section 8 - a section about pulling updates, downstream of +the decision it should inform. A real user hit exactly this. These tests +pin that the warning lives at the point of decision (adjacent to both +fork commands) and that /setup checks the origin's visibility BEFORE +writing anything, not in its closing notes. +""" +import re +import unittest +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +README = REPO / "README.md" +SETUP_GUIDE = REPO / "SETUP.md" +SETUP_COMMAND = REPO / ".claude" / "commands" / "setup.md" + + +def section(text: str, heading: str) -> str: + """Body of a markdown section up to the next heading of the same level.""" + level = heading.split(" ")[0] + pattern = re.compile( + rf"^{re.escape(heading)}\n(.*?)(?=^{level} |\Z)", re.MULTILINE | re.DOTALL + ) + match = pattern.search(text) + return match.group(1) if match else "" + + +class TestForkWarningsAtTheDecisionPoint(unittest.TestCase): + def assert_warns(self, body: str, where: str): + self.assertRegex( + body, + re.compile(r"public", re.IGNORECASE), + f"{where}'s fork section must say the fork will be public", + ) + self.assertIn( + "personal data", + body, + f"{where}'s fork section must say /setup writes personal data into tracked files", + ) + self.assertRegex( + body, + re.compile(r"section 8|§8|#8-pulling", re.IGNORECASE), + f"{where}'s fork section must point at SETUP.md section 8's private-remote recipe", + ) + + def test_readme_quick_start_warns_next_to_the_fork_command(self): + body = section(README.read_text(encoding="utf-8"), "### 1. Fork and clone") + self.assertIn("gh repo fork", body, "sanity: the fork command lives in this section") + self.assert_warns(body, "README") + + def test_setup_guide_warns_next_to_the_fork_command(self): + body = section(SETUP_GUIDE.read_text(encoding="utf-8"), "## 2. Fork and clone") + self.assertIn("gh repo fork", body, "sanity: the fork command lives in this section") + self.assert_warns(body, "SETUP.md") + + +class TestSetupChecksOriginBeforeWriting(unittest.TestCase): + def test_preflight_exists_and_precedes_profile_generation(self): + text = SETUP_COMMAND.read_text(encoding="utf-8") + self.assertIn( + "git remote get-url origin", + text, + "/setup must check where the working copy would publish to", + ) + preflight_at = text.index("git remote get-url origin") + writes_at = text.index("## Step 3: Generate Profile Files") + self.assertLess( + preflight_at, + writes_at, + "the origin check must run before any profile file is written - the " + "existing Step 4 note fires after everything is already on disk", + ) + self.assertIn( + "public", + text[max(0, preflight_at - 2000) : preflight_at + 2000].lower(), + "the preflight must be about public visibility, not just remote presence", + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_outcome_followup.py b/tests/test_outcome_followup.py new file mode 100755 index 0000000..ca9e1af --- /dev/null +++ b/tests/test_outcome_followup.py @@ -0,0 +1,90 @@ +"""Guards for /outcome's follow-up branch (Step 2b). + +The branch is part of the /outcome markdown spec (the spec IS the +implementation), so these tests pin the invariants that would break +silently: the draft-only rule (the branch must never send anything on the +user's behalf), the no-new-claims rule that keeps follow-ups inside the +framework's never-fabricate boundary, the two-follow-up cap that terminates +into /outcome's own no_response flow, and the deliberate contrast with +/gmail-sync's 30-day staleness flag. +""" +import subprocess +import sys +import unittest +from pathlib import Path + +try: + import yaml # noqa: F401 - only probing availability for the lint integration test + _HAVE_YAML = True +except ImportError: + _HAVE_YAML = False + +REPO = Path(__file__).resolve().parent.parent +COMMAND = REPO / ".claude" / "commands" / "outcome.md" + + +class OutcomeFollowupBranchSpec(unittest.TestCase): + def test_followup_branch_exists(self): + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "## Step 2b: Follow-Up Branch", + text, + "outcome.md lost its follow-up branch section", + ) + + def test_followup_argument_documented(self): + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "`followup `", + text, + "spec lost the /outcome followup argument forms", + ) + + def test_draft_only_rule_present(self): + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "draft only, never send", + text, + "spec lost the rule that the follow-up branch never sends anything on the user's behalf", + ) + + def test_no_new_claims_rule_present(self): + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "no new claims", + text, + "spec lost the rule that follow-ups only reuse claims from the archived submitted materials", + ) + + def test_two_followup_cap_present(self): + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "Maximum two follow-ups per application", + text, + "spec lost the cap that stops the follow-up branch from nagging indefinitely", + ) + + def test_threshold_contrast_with_gmail_sync_documented(self): + text = COMMAND.read_text(encoding="utf-8") + self.assertIn( + "30-day staleness flag", + text, + "spec lost the rationale for the 10-day nudge vs /gmail-sync's 30-day alarm", + ) + + @unittest.skipUnless( + _HAVE_YAML, + "PyYAML not installed (the CI Python-test job omits it; the lint job runs lint_skills.py directly)", + ) + def test_lint_skills_passes(self): + result = subprocess.run( + [sys.executable, str(REPO / "tools" / "lint_skills.py")], + cwd=REPO, + capture_output=True, + text=True, + ) + self.assertEqual(result.returncode, 0, f"lint_skills.py failed:\n{result.stdout}{result.stderr}") + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_placeholder_integrity.py b/tests/test_placeholder_integrity.py new file mode 100755 index 0000000..2e04d1c --- /dev/null +++ b/tests/test_placeholder_integrity.py @@ -0,0 +1,92 @@ +"""Guards for CI's placeholder-integrity sentinels. + +The job exists to catch personal data committed to the upstream template. +That only works when each sentinel sits IN the data /setup replaces: the +CV's old sentinel was `[YOUR_NAME]`, whose only occurrences were a header +comment and the hyperref pdftitle - /setup's documented edit ("replace +placeholder personal data with their actual name, contact info") touches +neither, so a fully personalized CV with a real name, address, phone and +email passed the check (review finding F28, 2026-08-19; proven +empirically). Same weakness for 01-candidate-profile.md's `