Add test suite for generate.py by breferrari · Pull Request #12 · PackWeave/registry

breferrari · 2026-03-25T08:50:49Z

Summary

Add 34 pytest tests covering all generate.py functions: parsing, validation, version immutability, index regeneration, and end-to-end generation
Update validate.yml to run tests on PRs alongside validation
Depends on Replace regex TOML parsing with stdlib tomllib #10 (tomllib) and Add pack validation to generate.py and CI #11 (validation) — includes both via cherry-pick

Test plan

All 34 tests pass locally
Tests use tmp_path for full isolation — no filesystem side effects
CI passes on this PR

Closes #7

🤖 Generated with Claude Code

The regex-based parse_pack_toml() worked for simple flat fields but would break on multiline strings, inline tables, or other valid TOML constructs. Switch to Python's stdlib tomllib (3.11+) for spec-compliant parsing. Pin Python 3.12 in CI via actions/setup-python to guarantee tomllib availability regardless of runner image updates. Output is byte-for-byte identical to the regex-based parser for all 13 current packs. Closes #5 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add validate_pack() and validate_unique_server_names() that check: - Required fields (name, version, description) - Name format (lowercase, digits, hyphens, no trailing hyphen) - Semver format (X.Y.Z) - authors/keywords are string lists - [[servers]] syntax (not [servers]) - Server names: format, uniqueness within and across packs - Server command/url required based on transport type Validation runs before generation on every invocation. Add --validate-only flag for local/CI use without writing files. Add validate.yml workflow that runs on PRs touching src/** or scripts/generate.py so contributors get feedback before merge. Closes #6 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add 34 pytest tests covering: - TOML parsing (valid, missing section, optional fields) - Semver sorting and fallback - File map building - Pack validation (all rules: names, versions, servers, transports) - Cross-pack server name uniqueness - Version immutability enforcement - Index regeneration edge cases - Full end-to-end generation with multi-version accumulation Update validate.yml to install pytest and run tests on PRs. Closes #7 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Adds a pytest suite for scripts/generate.py and wires it into CI so pack parsing/validation and registry generation behavior is continuously verified.

Changes:

Introduce scripts/test_generate.py with end-to-end and unit coverage for parsing, validation, index regeneration, and version immutability.
Update scripts/generate.py to use tomllib, add pack validation + cross-pack server-name uniqueness checks, and add a --validate-only mode.
Add a PR validation workflow running --validate-only and pytest; update publish workflow to use Python 3.12.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
scripts/test_generate.py	New pytest suite exercising generate/validate/index behaviors via isolated temp dirs.
scripts/generate.py	Switch TOML parsing to `tomllib`, add validation helpers, and introduce a validation pass in `main()`.
.github/workflows/validate.yml	New PR workflow to validate packs and run the new tests.
.github/workflows/publish.yml	Ensure Python 3.12 is used when regenerating registry artifacts on main.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T08:56:08Z

+        except (ValueError, tomllib.TOMLDecodeError):
+            continue
+
+        for server in data.get("servers", []):


validate_unique_server_names() iterates for server in data.get("servers", []) and then calls server.get(...). If a pack uses [servers] (dict) or has non-table entries, this will crash with AttributeError during the cross-pack pass (even though validate_pack() would have reported the issue). Guard that servers is a list of dicts (or reuse the same structure checks as validate_pack()), and skip/record an error when it isn’t.

Suggested change

for server in data.get("servers", []):

servers = data.get("servers")

# If there is no [servers] / [[servers]] section, nothing to validate.

if servers is None:

continue

# Guard against packs that use [servers] (a table/dict) instead of

# [[servers]] (an array of tables), or otherwise mis-type this field.

if not isinstance(servers, list):

errors.append(

f"{pack_dir.name}: 'servers' must be an array of tables ([[servers]])"

)

continue

for server in servers:

# Skip and record an error for any non-table entries.

if not isinstance(server, dict):

errors.append(

f"{pack_dir.name}: each entry in 'servers' must be a table"

)

continue

Copilot · 2026-03-25T08:56:08Z

+from generate import (
+    build_files_map,
+    parse_pack_toml,
+    parse_pack_toml_full,
+    process_pack,
+    regenerate_index,
+    semver_key,
+    validate_pack,
+    validate_unique_server_names,
+)


parse_pack_toml_full is imported but never used in this test module. Removing the unused import will keep the test file clean and avoid failing linting if it’s added later.

Copilot · 2026-03-25T08:56:08Z

+        run: python3 scripts/generate.py --validate-only
+
+      - name: Install test dependencies
+        run: pip install pytest


This workflow mixes python3/python and uses bare pip. On GitHub-hosted runners these usually point to the same interpreter, but it’s not guaranteed and can lead to installing pytest into a different environment than the one running tests. Consider consistently using a single interpreter (e.g., python -m pip install ..., python scripts/generate.py ..., python -m pytest ...).

Suggested change

run: python3 scripts/generate.py --validate-only

- name: Install test dependencies

run: pip install pytest

run: python scripts/generate.py --validate-only

- name: Install test dependencies

run: python -m pip install pytest

Copilot · 2026-03-25T08:56:09Z

+    with open(pack_toml_path, "rb") as f:
+        data = tomllib.load(f)
+    if "pack" not in data:
+        raise ValueError("No [pack] section found in pack.toml")


parse_pack_toml_full() only checks that the top-level key pack exists, but doesn’t verify it’s a TOML table (dict). If someone has a syntactically-valid TOML like pack = "foo", validate_pack() / parse_pack_toml() will later crash when calling .get() on a non-dict. Consider validating isinstance(data.get("pack"), dict) here and raising a ValueError with a clear message when it isn’t a table.

Suggested change

raise ValueError("No [pack] section found in pack.toml")

raise ValueError("No [pack] section found in pack.toml")

if not isinstance(data.get("pack"), dict):

raise ValueError("[pack] section in pack.toml must be a table")

Copilot · 2026-03-25T08:56:09Z

+    pack = data["pack"]
+
+    # Required fields
+    for field in ("name", "version", "description"):
+        if field not in pack or not pack[field]:
+            errors.append(f"{pack_dir_name}: missing required field '{field}' in [pack]")
+
+    name = pack.get("name", "")
+    if name and not _NAME_RE.match(name):
+        errors.append(
+            f"{pack_dir_name}: invalid pack name '{name}' — "
+            "must be lowercase letters, digits, and hyphens only"
+        )
+    if name and name != pack_dir_name:
+        errors.append(
+            f"{pack_dir_name}: pack name '{name}' does not match directory name"
+        )
+
+    version = pack.get("version", "")
+    if version and not _SEMVER_RE.match(version):
+        errors.append(
+            f"{pack_dir_name}: invalid version '{version}' — must be X.Y.Z"
+        )


validate_pack() assumes pack is a dict of strings (e.g., _SEMVER_RE.match(version) and _NAME_RE.match(name)), but TOML can legally encode non-string types (ints/bools). With non-string values this will raise TypeError instead of returning validation errors. Add explicit type checks/coercion for required fields (name, version, description) and treat non-strings as validation errors.

- Add isinstance check for [pack] section (must be a table, not scalar) - Add string type checks for required fields (name, version, description) - Guard server entries with isinstance(server, dict) checks - Harden validate_unique_server_names() against non-list/non-dict servers - Skip cross-pack validation when per-pack errors already exist - Use consistent `python` (not `python3`) in CI workflows - Add scripts/generate.py to publish.yml path triggers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

breferrari and others added 3 commits March 25, 2026 09:47

Copilot AI review requested due to automatic review settings March 25, 2026 08:50

Copilot started reviewing on behalf of breferrari March 25, 2026 08:51 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

breferrari and others added 2 commits March 26, 2026 07:21

chore: add __pycache__ to .gitignore

1a411f5

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add test suite for generate.py#12

Add test suite for generate.py#12
breferrari wants to merge 5 commits into
mainfrom
feat/add-tests

breferrari commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        for server in data.get("servers", []):
+        servers = data.get("servers")
+        # If there is no [servers] / [[servers]] section, nothing to validate.
+        if servers is None:
+            continue
+        # Guard against packs that use [servers] (a table/dict) instead of
+        # [[servers]] (an array of tables), or otherwise mis-type this field.
+        if not isinstance(servers, list):
+            errors.append(
+                f"{pack_dir.name}: 'servers' must be an array of tables ([[servers]])"
+            )
+            continue
+        for server in servers:
+            # Skip and record an error for any non-table entries.
+            if not isinstance(server, dict):
+                errors.append(
+                    f"{pack_dir.name}: each entry in 'servers' must be a table"
+                )
+                continue

Uh oh!

Conversation

breferrari commented Mar 25, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants