diff --git a/.claude/commands/commit.md b/.claude/commands/commit.md deleted file mode 100644 index 0091d8b..0000000 --- a/.claude/commands/commit.md +++ /dev/null @@ -1,229 +0,0 @@ ---- -description: Create a git commit following project standards and safety protocols -allowed-tools: Bash(git status:*), Bash(git log:*), Bash(git add:*), Bash(git diff:*), Bash(git commit:*), Bash(make test:*) ---- - -# commit - -Create a git commit following all project standards and safety protocols for pgxntool-test. - -**CRITICAL REQUIREMENTS:** - -1. **Git Safety**: Never update `git config`, never force push to `main`/`master`, never skip hooks unless explicitly requested - -2. **Commit Attribution**: Do NOT add "Generated with Claude Code" to commit message body. The standard Co-Authored-By trailer is acceptable per project CLAUDE.md. - -3. **Multi-Repo Commits**: If BOTH repositories have changes, commit BOTH (unless user explicitly says otherwise). Do NOT create empty commits - only commit repos with actual changes. - -4. **Testing**: ALL tests must pass before committing: - - Tests are run via test subagent (first step in workflow) - - Check subagent output carefully for any "not ok" lines - - **If ANY tests fail: STOP. Do NOT commit. Ask the user what to do.** - - There is NO such thing as an "acceptable" failing test - - Do NOT rationalize failures as "pre-existing" or "unrelated" - -**WORKFLOW:** - -1. **Launch test subagent** (unless user explicitly says not to): - - Use Task tool to launch test subagent to run `make test` - - Run in background so we can continue with analysis - - Tests will be checked before committing - -2. **While tests run**, gather information in parallel: - - `git status`, `git diff --stat`, `git log -10 --oneline` for both repos - -3. **Analyze changes** in BOTH repositories and draft commit messages for BOTH: - - **CRITICAL: Item Ordering** - - Order all items (changes, bullet points) by **decreasing importance** - - Importance = impact of the change × likelihood someone reading history will care - - Most impactful/interesting changes first, minor details last - - Think: "What would I want to see first when reading this in git log 2 years from now?" - - For pgxntool: - - Analyze: `git status`, `git diff --stat`, `git log -10 --oneline` - - Draft message with structure: - ``` - Subject line - - [Main changes in pgxntool, ordered by decreasing importance...] - - Related changes in pgxntool-test: - - [RELEVANT test change 1] - - [RELEVANT test change 2] - - Co-Authored-By: Claude - ``` - - Only mention RELEVANT test changes (1-3 bullets): - * ✅ Include: Tests for new features, template updates, user docs - * ❌ Exclude: Test refactoring, infrastructure, internal changes - - Wrap code references in backticks (e.g., `helpers.bash`, `make test`) - - No hash placeholder needed - pgxntool doesn't reference test hash - - For pgxntool-test: - - Analyze: `git status`, `git diff --stat`, `git log -10 --oneline` - - Draft message with structure: - ``` - Subject line - - Add tests/updates for pgxntool commit [PGXNTOOL_COMMIT_HASH] (brief description): - - [Key pgxntool change 1] - - [Key pgxntool change 2] - - [pgxntool-test specific changes, ordered by decreasing importance...] - - Co-Authored-By: Claude - ``` - - Use placeholder `[PGXNTOOL_COMMIT_HASH]` - - Include brief summary (2-3 bullets) of pgxntool changes near top - - Wrap code references in backticks - - **If only one repo has changes:** - Skip the repo with no changes. In the commit message for the repo that has changes, - add: "Changes only in [repo]. No related changes in [other repo]." before Co-Authored-By. - -4. **PRESENT both proposed commit messages to the user and WAIT for approval** - - Show both messages: - ``` - ## Proposed Commit for pgxntool: - [message] - - ## Proposed Commit for pgxntool-test: - [message with [PGXNTOOL_COMMIT_HASH] placeholder] - ``` - - **Note:** Mention any files that are intentionally not being committed and why. - - **Note:** If only one repo has changes, show only that message (with note about other repo). - -5. **Wait for tests to complete** - THIS IS MANDATORY: - - Check test subagent output for completion - - Verify ALL tests passed (no "not ok" lines) - - **If ANY tests fail: STOP. Do NOT commit. Ask the user what to do.** - - There is NO such thing as an "acceptable" failing test - - Do NOT rationalize failures as "pre-existing" or "unrelated" - - Only proceed to commit if ALL tests pass - -6. **After tests pass AND receiving approval, execute two-phase commit:** - - **Phase 1: Commit pgxntool** - - a. `cd ../pgxntool` - - b. Stage changes: `git add` (include ALL new files per guidelines below) - - Check `git status` for untracked files - - ALL untracked files that are part of the feature/change MUST be staged - - New scripts, new documentation, new helper files, etc. should all be included - - Do NOT leave new files uncommitted unless explicitly told to exclude them - - c. Verify staged files: `git status` - - Confirm ALL modified AND untracked files are staged - - STOP and ask user if staging doesn't match intent - - d. Commit using approved message: - ```bash - git commit -m "$(cat <<'EOF' - [approved pgxntool message] - EOF - )" - ``` - - e. Capture hash: `PGXNTOOL_HASH=$(git log -1 --format=%h)` - - f. Verify: `git status` - - g. Handle pre-commit hooks if needed: - - Check if hooks modified files - - Check authorship: `git log -1 --format='%an %ae'` - - Check branch status - - Amend if safe or create new commit - - **Phase 2: Commit pgxntool-test** - - a. `cd ../pgxntool-test` - - b. Replace `[PGXNTOOL_COMMIT_HASH]` in approved message with `$PGXNTOOL_HASH` - - Keep everything else EXACTLY the same - - c. Stage changes: `git add` (include ALL new files) - - d. Verify staged files: `git status` - - e. Commit using hash-injected message: - ```bash - git commit -m "$(cat <<'EOF' - [approved message with actual pgxntool hash] - EOF - )" - ``` - - f. Capture hash: `TEST_HASH=$(git log -1 --format=%h)` - - g. Verify: `git status` - - h. Handle pre-commit hooks if needed - - **Note:** If only one repo has changes, skip the phase for the other repo. - -**MULTI-REPO COMMIT CONTEXT:** - -The two repositories: -- **pgxntool** - The main framework -- **pgxntool-test** - Test harness (includes template files in `template/` directory) - -When committing changes that span repositories: - -1. **pgxntool-test commit MUST reference pgxntool commit hash** - - pgxntool commit format: - ``` - Subject line - - [Main changes...] - - Related changes in pgxntool-test: - - [RELEVANT test change] - - [Keep to 1-3 bullets] - - Co-Authored-By: Claude - ``` - - pgxntool-test commit format: - ``` - Subject line - - Add tests for pgxntool commit def5678 (brief description): - - [Key pgxntool change 1] - - [Key pgxntool change 2] - - [pgxntool-test specific changes...] - - Co-Authored-By: Claude - ``` - -2. **Relevance filter for pgxntool message:** - - ✅ Include: Tests for new features, template updates, user documentation - - ❌ Exclude: Test refactoring, infrastructure changes, internal improvements - - Keep it brief (1-3 bullets max) - -3. **Commit workflow:** - - Commit pgxntool first (no placeholder) - - Capture pgxntool hash - - Commit pgxntool-test (inject pgxntool hash) - - Result: pgxntool-test references pgxntool commit - -4. **Single-repo case:** - Add line: "Changes only in [repo]. No related changes in [other repo]." - -**REPOSITORY CONTEXT:** - -This is pgxntool, a PostgreSQL extension build framework. Key facts: -- Main Makefile is `base.mk` -- Scripts live in root directory -- Documentation is in `README.asc` (generates `README.html`) - -**RESTRICTIONS:** -- DO NOT push unless explicitly asked -- DO NOT commit files with actual secrets (`.env`, `credentials.json`, etc.) -- Never use `-i` flags (`git commit -i`, `git rebase -i`, etc.) diff --git a/.claude/settings.json b/.claude/settings.json index 134b554..fd12d9a 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -13,7 +13,27 @@ "Bash(DEBUG=3 test/bats/bin/bats:*)", "Bash(DEBUG=4 test/bats/bin/bats:*)", "Bash(DEBUG=5 test/bats/bin/bats:*)", - "Edit" + "Bash(bash .claude/skills/commit/scripts/*)", + "Bash(find:*)", + "Bash(gh pr list:*)", + "Bash(gh pr view:*)", + "Bash(gh repo view:*)", + "Bash(git checkout:*)", + "Bash(git fetch:*)", + "Bash(git ls-tree:*)", + "Bash(git merge:*)", + "Bash(git rebase:*)", + "Bash(git rev-parse:*)", + "Bash(git worktree:*)", + "Bash(grep:*)", + "Bash(ls:*)", + "Bash(make html:*)", + "Bash(make test-extra:*)", + "Bash(make test:*)", + "Bash(readlink:*)", + "Bash(tee:*)", + "Edit", + "Skill(building-claude-code-skills)" ], "additionalDirectories": [ "/tmp/", diff --git a/.claude/skills/commit/SKILL.md b/.claude/skills/commit/SKILL.md new file mode 100644 index 0000000..1e6a3ea --- /dev/null +++ b/.claude/skills/commit/SKILL.md @@ -0,0 +1,119 @@ +--- +name: commit +description: | + Create git commits across pgxntool and pgxntool-test repositories following project + standards. Preprocesses repo info, runs tests, drafts cross-referenced commit messages, + and executes two-phase commits with hash injection. + + Use when: "commit", "create commit", "commit changes", "/commit" +allowed-tools: Bash(git status:*), Bash(git log:*), Bash(git add:*), Bash(git diff:*), Bash(git commit:*), Bash(git branch:*), Bash(bash .claude/skills/commit/scripts/*), Bash(make test:*), Read, Edit, Task +--- + +# Commit Skill + +Create git commits following project standards and safety protocols for pgxntool-test. + +## Critical Requirements + +| Rule | Details | +|------|---------| +| **Git Safety** | Never update git config. Never force push. Never skip hooks unless requested. | +| **Attribution** | No "Generated with Claude Code" in body. Co-Authored-By trailer is OK. | +| **Multi-Repo** | Commit BOTH repos if both have changes (unless told otherwise). No empty commits. | +| **Testing** | ALL tests must pass. ANY failure = STOP and ask user. No rationalizing failures. | +| **HISTORY.asc** | Update for significant user-visible pgxntool changes. Propose entry, get confirmation. | + +## Workflow + +### 1. Launch Tests (Background) + +Use Task tool to launch test subagent to run `make test` in background. +Skip only if user explicitly says to. + +### 2. Gather Repository Info + +Run the preprocessing script to collect all git data in one call: + +```bash +bash .claude/skills/commit/scripts/gather-repo-info.sh ../pgxntool . +``` + +Review output. Identify which repos have changes. + +### 3. Check HISTORY.asc (pgxntool changes only) + +Read `../pgxntool/HISTORY.asc`. Determine if changes are significant and user-visible: +- Yes: New features, behavior changes, bug fixes users would notice +- No: Internal refactoring, test changes, cleanup, documentation fixes + +If update needed: +1. Propose entry in AsciiDoc format (use `==` for heading) +2. Ask for confirmation BEFORE proceeding +3. Add to STABLE section at TOP of file (create section if missing): + ``` + STABLE + ------ + == [Entry heading] + [Entry description] + + [existing content...] + ``` + +### 4. Draft Commit Messages + +Read the format guide for detailed templates and rules: +**`.claude/skills/commit/guides/commit-message-format.md`** + +Key principles: +- Order items by **decreasing importance** (impact x likelihood someone cares) +- pgxntool message includes relevant test changes (1-3 bullets) +- pgxntool-test message uses `[PGXNTOOL_COMMIT_HASH]` placeholder +- Single-repo: note "Changes only in [repo]. No related changes in [other repo]." +- Wrap code references in backticks + +### 5. Present for Approval + +Show proposed commit messages. Wait for user approval. +Mention any files intentionally excluded and why. +If only one repo has changes, show only that message (with note about other repo). + +### 6. Verify Tests Passed (MANDATORY) + +Check test subagent output for completion. +- Look for ANY "not ok" lines +- **If tests fail: STOP. Do NOT commit. Ask user what to do.** +- There is NO such thing as an "acceptable" failing test +- Do NOT rationalize failures as "pre-existing" or "unrelated" +- Only proceed if ALL tests pass + +### 7. Execute Two-Phase Commit + +After tests pass AND receiving user approval: + +1. **Phase 1: Commit pgxntool** (if it has changes) + - Stage files: `git add` specific files (include ALL new files for the feature) + - Verify staging: `git status` (STOP if mismatch) + - Commit with approved message using HEREDOC + - Capture hash: `PGXNTOOL_HASH=$(git log -1 --format=%h)` + - Verify: `git status` + +2. **Phase 2: Commit pgxntool-test** (if it has changes) + - Replace `[PGXNTOOL_COMMIT_HASH]` with captured hash + - Stage, verify, commit, verify (same pattern) + +For detailed execution steps including pre-commit hook handling, read: +**`.claude/skills/commit/guides/commit-message-format.md`** -> "Two-Phase Commit Execution" + +Always use HEREDOC format: +```bash +git commit -m "$(cat <<'EOF' +[message] +EOF +)" +``` + +## Restrictions + +- DO NOT push unless explicitly asked +- DO NOT commit files with secrets (.env, credentials.json) +- Never use `-i` flags (git commit -i, git rebase -i) diff --git a/.claude/skills/commit/guides/commit-message-format.md b/.claude/skills/commit/guides/commit-message-format.md new file mode 100644 index 0000000..ee8b8cd --- /dev/null +++ b/.claude/skills/commit/guides/commit-message-format.md @@ -0,0 +1,123 @@ +# Commit Message Format Guide + +## Item Ordering (CRITICAL) + +Order all items (changes, bullet points) by **decreasing importance**: +- Importance = impact of the change x likelihood someone reading history will care +- Most impactful/interesting changes first, minor details last +- Think: "What would I want to see first when reading this in git log 2 years from now?" + +## pgxntool Commit Format + +``` +Subject line + +[Main changes in pgxntool, ordered by decreasing importance...] + +Related changes in pgxntool-test: +- [RELEVANT test change 1] +- [RELEVANT test change 2] + +Co-Authored-By: Claude +``` + +**Relevance filter for test changes:** + +| Include | Exclude | +|---------|---------| +| Tests for new features | Test refactoring | +| Template updates | Infrastructure changes | +| User documentation | Internal improvements | + +Keep to 1-3 bullets max. Wrap code references in backticks (e.g., `helpers.bash`, `make test`). +No hash placeholder needed - pgxntool doesn't reference test hash. + +## pgxntool-test Commit Format + +``` +Subject line + +Add tests/updates for pgxntool commit [PGXNTOOL_COMMIT_HASH] (brief description): +- [Key pgxntool change 1] +- [Key pgxntool change 2] + +[pgxntool-test specific changes, ordered by decreasing importance...] + +Co-Authored-By: Claude +``` + +- Use placeholder `[PGXNTOOL_COMMIT_HASH]` (replaced with actual hash during execution) +- Include brief summary (2-3 bullets) of pgxntool changes near top +- Wrap code references in backticks + +## Single-Repo Case + +If only one repo has changes: +- Skip the repo with no changes +- In the commit message for the repo that has changes, add before Co-Authored-By: + ``` + Changes only in [repo]. No related changes in [other repo]. + ``` + +--- + +## Two-Phase Commit Execution + +### Phase 1: Commit pgxntool + +1. `cd ../pgxntool` +2. Stage changes: `git add` specific files + - Check `git status` for untracked files + - ALL untracked files that are part of the feature/change MUST be staged + - New scripts, documentation, helper files should all be included + - Do NOT leave new files uncommitted unless explicitly told to exclude them +3. Verify staged files: `git status` + - Confirm ALL modified AND untracked files are staged + - STOP and ask user if staging doesn't match intent +4. Commit: + ```bash + git commit -m "$(cat <<'EOF' + [approved pgxntool message] + EOF + )" + ``` +5. Capture hash: `PGXNTOOL_HASH=$(git log -1 --format=%h)` +6. Verify: `git status` +7. Handle pre-commit hooks if needed: + - Check if hooks modified files + - Check authorship: `git log -1 --format='%an %ae'` + - Check branch status + - Amend if safe or create new commit + +### Phase 2: Commit pgxntool-test + +1. Return to pgxntool-test directory +2. Replace `[PGXNTOOL_COMMIT_HASH]` in approved message with `$PGXNTOOL_HASH` + - Keep everything else EXACTLY the same +3. Stage changes: `git add` specific files (include ALL new files) +4. Verify staged files: `git status` +5. Commit: + ```bash + git commit -m "$(cat <<'EOF' + [approved message with actual pgxntool hash] + EOF + )" + ``` +6. Capture hash: `TEST_HASH=$(git log -1 --format=%h)` +7. Verify: `git status` +8. Handle pre-commit hooks if needed + +### Important Notes + +- If only one repo has changes, skip the phase for the other repo +- Always use HEREDOC format for commit messages +- Never use `-i` flags (git commit -i, git rebase -i) + +## Repository Context + +The two repositories: +- **pgxntool** (`../pgxntool/`) - The framework being tested. Main Makefile is `base.mk`, scripts in root, docs in `README.asc`. +- **pgxntool-test** (this repo) - Test harness with template files in `template/` directory. + +Commit pgxntool first (no placeholder), capture hash, then commit pgxntool-test (inject hash). +Result: pgxntool-test commit references the pgxntool commit it corresponds to. diff --git a/.claude/skills/commit/scripts/gather-repo-info.sh b/.claude/skills/commit/scripts/gather-repo-info.sh new file mode 100755 index 0000000..487021c --- /dev/null +++ b/.claude/skills/commit/scripts/gather-repo-info.sh @@ -0,0 +1,75 @@ +#!/bin/bash +set -e + +# gather-repo-info.sh - Collect git info for commit preparation +# +# Gathers branch, status, diff stats, and recent log for each repo in one +# call, reducing multiple git commands to a single preprocessed summary. +# +# Usage: gather-repo-info.sh [ ...] +# Output: Structured text summary per repository + +gather_info() { + local repo_path="$1" + local repo_name + repo_name=$(basename "$(cd "$repo_path" && pwd)") + + echo "## $repo_name" + echo "Path: $(cd "$repo_path" && pwd)" + + ( + cd "$repo_path" || exit 1 + + echo "Branch: $(git branch --show-current 2>/dev/null || echo 'detached')" + echo + + local status + status=$(git status -s 2>/dev/null) + if [ -z "$status" ]; then + echo "Status: CLEAN (no changes)" + echo + return + fi + + echo "### Changes" + echo "$status" + echo + + local diff_stat + diff_stat=$(git diff --stat 2>/dev/null) + if [ -n "$diff_stat" ]; then + echo "### Unstaged Diff" + echo "$diff_stat" + echo + fi + + local cached_stat + cached_stat=$(git diff --cached --stat 2>/dev/null) + if [ -n "$cached_stat" ]; then + echo "### Staged Diff" + echo "$cached_stat" + echo + fi + + echo "### Recent Commits" + git log -10 --oneline 2>/dev/null + echo + ) +} + +if [ $# -eq 0 ]; then + echo "Usage: gather-repo-info.sh [ ...]" >&2 + exit 1 +fi + +for repo in "$@"; do + if ! git -C "$repo" rev-parse --git-dir &>/dev/null; then + echo "## $(basename "$repo")" + echo "WARNING: Not a git repository: $repo" + echo + continue + fi + gather_info "$repo" + echo "---" + echo +done diff --git a/CLAUDE.md b/CLAUDE.md index f672342..354b50d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,8 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co Subagents are automatically discovered and loaded at session start from: - `.claude/agents/*.md` - Specialized domain experts (invoked via Task tool) -- `.claude/commands/*.md` - Command/skill handlers (invoked via Skill tool) +- `.claude/skills/*/SKILL.md` - Skills with preprocessing scripts and guides (invoked via Skill tool) +- `.claude/commands/*.md` - Simple command handlers (invoked via Skill tool) These subagents are already available in your context - you don't need to discover them. Just USE them whenever their expertise is relevant. @@ -26,9 +27,10 @@ These subagents are already available in your context - you don't need to discov **At the start of every session**: Invoke the pgtle subagent to check if there are any newer versions of pg_tle than what it has already analyzed. If new versions exist, the subagent should analyze them for API changes and update its knowledge of version boundaries. -## Claude Commands +## Claude Skills and Commands -The `/commit` Claude Command lives in this repository (`.claude/commands/commit.md`). pgxntool no longer has its own copy. +The `/commit` skill lives in `.claude/skills/commit/` with a preprocessing script and format guide. +Other commands (worktree, pr, pgxntool-update) remain in `.claude/commands/`. ## What This Repo Is @@ -103,7 +105,7 @@ pgxntool-test/ ├── template/ # Template extension files for test repos ├── tests/ # Test suite (see test subagent for details) ├── test/bats/ # BATS framework (git submodule) -├── .claude/ # Claude subagents and commands +├── .claude/ # Claude subagents, skills, and commands └── .envs/ # Test environments (gitignored) ``` diff --git a/Makefile b/Makefile index 2ecbab7..cb45b71 100644 --- a/Makefile +++ b/Makefile @@ -110,7 +110,8 @@ clean: clean-envs # Note: This works on the pgxntool source repository, not test environments ASCIIDOC_CMD := $(shell which asciidoctor 2>/dev/null || which asciidoc 2>/dev/null) PGXNTOOL_SOURCE_DIR := $(shell cd $(CURDIR)/../pgxntool && pwd) -.PHONY: readme +.PHONY: html readme +html: readme readme: ifndef ASCIIDOC_CMD $(error Could not find "asciidoc" or "asciidoctor". Add one of them to your PATH) diff --git a/relative-out/1-test_that_does_cd.log b/relative-out/1-test_that_does_cd.log new file mode 100644 index 0000000..4c73f68 --- /dev/null +++ b/relative-out/1-test_that_does_cd.log @@ -0,0 +1 @@ +yey from test directory diff --git a/template/test/expected/verify_install_marker.out b/template/test/expected/verify_install_marker.out new file mode 100644 index 0000000..2a555bb --- /dev/null +++ b/template/test/expected/verify_install_marker.out @@ -0,0 +1,26 @@ +-- ============================================================================ +-- Verify test/install state persists into the main test suite +-- ============================================================================ +-- +-- DO NOT MODIFY THIS FILE WITHOUT EXPLICIT HUMAN APPROVAL. +-- +-- This test exists to catch a specific, subtle bug: if test/install and the +-- main test suite run in separate pg_regress invocations, pg_regress drops +-- and recreates the database between them, destroying everything test/install +-- set up. The ONLY way test/install state can persist is if install files and +-- regular test files run within a SINGLE pg_regress invocation (via a schedule +-- file that lists install files first). +-- +-- If this test fails with "relation pgxntool_install_marker does not exist", +-- it means the test-install implementation is broken: it's running install +-- files in a separate pg_regress invocation instead of the same one. +-- +-- This file is intentionally minimal to prevent well-meaning "improvements" +-- from masking the failure it's designed to detect. +-- ============================================================================ +SELECT marker FROM pgxntool_install_marker; + marker +-------- + alive +(1 row) + diff --git a/template/test/install/.gitignore b/template/test/install/.gitignore new file mode 100644 index 0000000..cc779d7 --- /dev/null +++ b/template/test/install/.gitignore @@ -0,0 +1,2 @@ +# pg_regress artifacts +*.out.diff diff --git a/template/test/install/create_install_marker.out b/template/test/install/create_install_marker.out new file mode 100644 index 0000000..6ed7d30 --- /dev/null +++ b/template/test/install/create_install_marker.out @@ -0,0 +1,2 @@ +CREATE TABLE pgxntool_install_marker (marker text); +INSERT INTO pgxntool_install_marker VALUES ('alive'); diff --git a/template/test/install/create_install_marker.sql b/template/test/install/create_install_marker.sql new file mode 100644 index 0000000..6ed7d30 --- /dev/null +++ b/template/test/install/create_install_marker.sql @@ -0,0 +1,2 @@ +CREATE TABLE pgxntool_install_marker (marker text); +INSERT INTO pgxntool_install_marker VALUES ('alive'); diff --git a/template/test/sql/verify_install_marker.sql b/template/test/sql/verify_install_marker.sql new file mode 100644 index 0000000..ec5a470 --- /dev/null +++ b/template/test/sql/verify_install_marker.sql @@ -0,0 +1,21 @@ +-- ============================================================================ +-- Verify test/install state persists into the main test suite +-- ============================================================================ +-- +-- DO NOT MODIFY THIS FILE WITHOUT EXPLICIT HUMAN APPROVAL. +-- +-- This test exists to catch a specific, subtle bug: if test/install and the +-- main test suite run in separate pg_regress invocations, pg_regress drops +-- and recreates the database between them, destroying everything test/install +-- set up. The ONLY way test/install state can persist is if install files and +-- regular test files run within a SINGLE pg_regress invocation (via a schedule +-- file that lists install files first). +-- +-- If this test fails with "relation pgxntool_install_marker does not exist", +-- it means the test-install implementation is broken: it's running install +-- files in a separate pg_regress invocation instead of the same one. +-- +-- This file is intentionally minimal to prevent well-meaning "improvements" +-- from masking the failure it's designed to detect. +-- ============================================================================ +SELECT marker FROM pgxntool_install_marker; diff --git a/test/bats b/test/bats index cfdec8f..509e2d7 160000 --- a/test/bats +++ b/test/bats @@ -1 +1 @@ -Subproject commit cfdec8ffec045351512e03d27679b12ce9cfed29 +Subproject commit 509e2d7f06cf0b2fde791539dee7245fd15ce56c diff --git a/test/lib/dist-expected-files.txt b/test/lib/dist-expected-files.txt index b00c4e7..81be30f 100644 --- a/test/lib/dist-expected-files.txt +++ b/test/lib/dist-expected-files.txt @@ -41,9 +41,17 @@ sql/pgxntool-test.sql # Test files (root level, copied from t/) test/ test/deps.sql +test/expected/ +test/expected/verify_install_marker.out test/input/ test/input/pgxntool-test.source +test/install/ +test/install/.gitignore +test/install/create_install_marker.out +test/install/create_install_marker.sql test/pgxntool +test/sql/ +test/sql/verify_install_marker.sql # pgxntool framework (the build system itself) pgxntool/ diff --git a/test/lib/foundation.bats b/test/lib/foundation.bats index bbd2b02..0943c6e 100644 --- a/test/lib/foundation.bats +++ b/test/lib/foundation.bats @@ -252,7 +252,7 @@ In a real extension, these would already exist before adding pgxntool." # Validate prerequisites before attempting git subtree # 1. Check PGXNREPO is accessible and safe - if [ ! -d "$PGXNREPO/.git" ]; then + if [ ! -e "$PGXNREPO/.git" ]; then # Not a local directory - must be a valid remote URL # Explicitly reject dangerous protocols first @@ -267,7 +267,7 @@ In a real extension, these would already exist before adding pgxntool." fi # 2. For local repos, verify branch exists - if [ -d "$PGXNREPO/.git" ]; then + if [ -e "$PGXNREPO/.git" ]; then if ! (cd "$PGXNREPO" && git rev-parse --verify "$PGXNBRANCH" >/dev/null 2>&1); then error "Branch $PGXNBRANCH does not exist in $PGXNREPO" fi @@ -276,7 +276,7 @@ In a real extension, these would already exist before adding pgxntool." # 3. Check if source repo is dirty and use rsync if needed # This matches the legacy test behavior in tests/clone local source_is_dirty=0 - if [ -d "$PGXNREPO/.git" ]; then + if [ -e "$PGXNREPO/.git" ]; then # SECURITY: rsync only works with local paths, never remote URLs if [[ "$PGXNREPO" == *://* ]]; then error "Cannot use rsync with remote URL: $PGXNREPO" diff --git a/test/lib/helpers.bash b/test/lib/helpers.bash index 18aee12..713d3c0 100644 --- a/test/lib/helpers.bash +++ b/test/lib/helpers.bash @@ -1411,8 +1411,9 @@ build_test_repo_from_template() { } # Check if pgxntool repo is dirty + # Note: Use -e instead of -d to handle git worktrees where .git is a file local source_is_dirty=0 - if [ -d "$PGXNREPO/.git" ]; then + if [ -e "$PGXNREPO/.git" ]; then if [ -n "$(cd "$PGXNREPO" && git status --porcelain)" ]; then source_is_dirty=1 local pgxn_branch diff --git a/test/standard/doc.bats b/test/standard/doc.bats index cdce724..64021e2 100755 --- a/test/standard/doc.bats +++ b/test/standard/doc.bats @@ -77,16 +77,16 @@ setup() { @test "documentation source files exist" { # OK to fail: ls returns non-zero if no files match, which would mean test should fail - local doc_files=$(ls "$TEST_DIR/doc_repo/doc"/*.adoc "$TEST_DIR/doc_repo/doc"/*.asciidoc 2>/dev/null || echo) + local doc_files=$(ls "$TEST_DIR/doc_repo/doc"/*.adoc "$TEST_DIR/doc_repo/doc"/*.asc "$TEST_DIR/doc_repo/doc"/*.asciidoc 2>/dev/null || echo) [ -n "$doc_files" ] } @test "ASCIIDOC='' make install does not create docs" { cd "$TEST_DIR/doc_repo" - # Remove any existing HTML files - local input=$(ls doc/*.adoc doc/*.asciidoc 2>/dev/null) - local expected=$(echo "$input" | sed -Ee 's/(adoc|asciidoc)$/html/') + # Remove any existing HTML files (all asciidoc extensions: adoc, asc, asciidoc) + local input=$(ls doc/*.adoc doc/*.asc doc/*.asciidoc 2>/dev/null) + local expected=$(echo "$input" | sed -Ee 's/(adoc|asc|asciidoc)$/html/') rm -f $expected # Install without ASCIIDOC (should fail, but we only care about HTML files not being created) @@ -108,9 +108,9 @@ setup() { @test "make test creates documentation" { cd "$TEST_DIR/doc_repo" - # Get expected HTML files - local input=$(ls doc/*.adoc doc/*.asciidoc 2>/dev/null) - local expected=$(echo "$input" | sed -Ee 's/(adoc|asciidoc)$/html/') + # Get expected HTML files (all asciidoc extensions: adoc, asc, asciidoc) + local input=$(ls doc/*.adoc doc/*.asc doc/*.asciidoc 2>/dev/null) + local expected=$(echo "$input" | sed -Ee 's/(adoc|asc|asciidoc)$/html/') # Run make test (may fail if PostgreSQL not running, but we only care about HTML generation) run make test diff --git a/test/standard/example.bats b/test/standard/example.bats new file mode 100644 index 0000000..59497d5 --- /dev/null +++ b/test/standard/example.bats @@ -0,0 +1,3 @@ +@test "test with spaces in the name" { + return 0 +} \ No newline at end of file diff --git a/test/standard/make-results-source-files.bats b/test/standard/make-results-source-files.bats index 5c5d0e3..b3d6b62 100644 --- a/test/standard/make-results-source-files.bats +++ b/test/standard/make-results-source-files.bats @@ -181,7 +181,8 @@ EOF # Run make results - it runs make test (which regenerates results), then copies results to expected # But it should NOT overwrite files that have output/*.source counterparts - run make results + # Disable verify-results since deps.sql has placeholder content that causes test failures + run make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results assert_success # The expected file should still have the source content (regenerated from output/*.source) @@ -203,7 +204,8 @@ EOF rm -f test/expected/pgxntool-test.out # Run make results - it runs make test (which regenerates results), then copies results to expected - run make results + # Disable verify-results since deps.sql has placeholder content that causes test failures + run make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results assert_success # Verify result file still exists and has content after make results (make test regenerated it) @@ -239,7 +241,8 @@ EOF rm -f test/expected/pgxntool-test.out # Run make results - it runs make test (which regenerates results), then copies results to expected - run make results + # Disable verify-results since deps.sql has placeholder content that causes test failures + run make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results assert_success # Non-source file should be copied from results diff --git a/test/standard/make-results.bats b/test/standard/make-results.bats index b8f7146..78fe09c 100755 --- a/test/standard/make-results.bats +++ b/test/standard/make-results.bats @@ -94,7 +94,8 @@ setup() { @test "make results updates expected output" { # Run make results to fix the expected output - run make results + # Disable verify-results since we intentionally created a mismatch to test this + run make PGXNTOOL_ENABLE_VERIFY_RESULTS=no results assert_success } diff --git a/test/standard/test-install-persistence.bats b/test/standard/test-install-persistence.bats new file mode 100644 index 0000000..c2e8aba --- /dev/null +++ b/test/standard/test-install-persistence.bats @@ -0,0 +1,61 @@ +#!/usr/bin/env bats + +# Test: test/install persistence - end-to-end validation +# +# This test verifies the CORE CONTRACT of test/install: that state created by +# test/install files persists into the main test suite. +# +# The template includes: +# test/install/create_install_marker.sql - Creates a table with a marker row +# test/sql/verify_install_marker.sql - Queries that table +# +# If test/install and regular tests run in the same pg_regress invocation +# (correct), the marker test passes. If they run in separate invocations +# (broken), the database gets dropped and recreated between them, and the +# marker test fails. + +load ../lib/helpers + +setup_file() { + setup_topdir + load_test_env "install-persistence" + ensure_foundation "$TEST_DIR" +} + +setup() { + load_test_env "install-persistence" + cd "$TEST_REPO" +} + +@test "template includes install marker files" { + assert_file_exists "test/install/create_install_marker.sql" + assert_file_exists "test/sql/verify_install_marker.sql" + assert_file_exists "test/expected/verify_install_marker.out" +} + +@test "test/install is auto-detected as enabled" { + # With test/install/ files present, schedule files should be generated + run make -n test 2>&1 + assert_success + echo "$output" | grep -q "schedule" +} + +@test "install marker state persists into main test suite" { + skip_if_no_postgres + + # pgxntool-test.sql is generated from a .source file and has no expected + # output file. pg_regress aborts if the expected file is missing, so create + # an empty one. (This is a pre-existing gap in the touch rule for .source tests.) + touch test/expected/pgxntool-test.out + + run make test + + # Verify the specific marker test produced results and passed. + # (pgxntool-test.sql is a template placeholder that always fails, so we + # can't just check for regression.diffs — we check the specific test.) + assert_file_exists test/results/verify_install_marker.out + run diff test/expected/verify_install_marker.out test/results/verify_install_marker.out + assert_success +} + +# vi: expandtab sw=2 ts=2 diff --git a/test/standard/test-test-build.bats b/test/standard/test-test-build.bats new file mode 100644 index 0000000..784e5cd --- /dev/null +++ b/test/standard/test-test-build.bats @@ -0,0 +1,197 @@ +#!/usr/bin/env bats + +# Test: test-build feature +# +# Tests that the test-build feature works correctly: +# - test-build runs when test/build/ directory exists +# - test-build can be disabled via PGXNTOOL_ENABLE_TEST_BUILD +# - test-build runs before regular tests when enabled + +load ../lib/helpers + +setup_file() { + # Set TOPDIR + setup_topdir + + # Independent test - gets its own isolated environment with foundation TEST_REPO + load_test_env "test-build" + ensure_foundation "$TEST_DIR" +} + +setup() { + load_test_env "test-build" + cd "$TEST_REPO" +} + +@test "test-build target does not exist when test/build/ is missing" { + # Remove test/build if it exists + rm -rf test/build + + # Check that test-build target is not in the list of available targets + run make list + [ "$status" -eq 0 ] + echo "$output" | grep -qv "test-build" +} + +@test "can create test/build directory" { + mkdir -p test/build + assert_dir_exists "test/build" +} + +@test "test-build target exists when test/build/ has SQL files" { + # Create a simple SQL file in test/build/ + cat > test/build/valid_test.sql <<'EOF' +-- Simple test to verify extension can be created +SELECT 1; +EOF + + # Check that test-build target is available + run make -n test-build 2>&1 + [ "$status" -eq 0 ] +} + +@test "test-build runs successfully with valid SQL" { + # valid_test.sql was created in previous test + # Create expected output file matching pg_regress format + # Note: pg_regress includes comments from SQL files in output + mkdir -p test/build/expected + cat > test/build/expected/valid_test.out <<'EOF' +-- Simple test to verify extension can be created +SELECT 1; + ?column? +---------- + 1 +(1 row) + +EOF + + # Run test-build + run make test-build + [ "$status" -eq 0 ] +} + +@test "test-build fails with invalid SQL" { + # Create an invalid SQL file + cat > test/build/invalid_test.sql <<'EOF' +-- This SQL has a syntax error +SELECT FROM nonexistent_table WHERE; +EOF + + # Create expected output file matching pg_regress format for the error + mkdir -p test/build/expected + cat > test/build/expected/invalid_test.out <<'EOF' +-- This SQL has a syntax error +SELECT FROM nonexistent_table WHERE; +ERROR: syntax error at or near ";" +LINE 1: SELECT FROM nonexistent_table WHERE; + ^ + +EOF + + # Run test-build - should fail because invalid_test.sql has a syntax error + # (even though valid_test.sql would pass) + run make test-build + [ "$status" -ne 0 ] + echo "$output" | grep -qE "(error|failed|regression.diffs)" +} + +@test "test-build can be disabled via PGXNTOOL_ENABLE_TEST_BUILD" { + # Ensure test/build exists + mkdir -p test/build + cat > test/build/disabled_test.sql <<'EOF' +SELECT 1; +EOF + + # Disable test-build and verify it's not in the list of available targets + run make list PGXNTOOL_ENABLE_TEST_BUILD= + [ "$status" -eq 0 ] + echo "$output" | grep -qv "test-build" +} + +@test "test target includes test-build when enabled" { + # Add another test file + cat > test/build/prereq_test.sql <<'EOF' +SELECT 1; +EOF + + # Create expected output file matching pg_regress format + mkdir -p test/build/expected + cat > test/build/expected/prereq_test.out <<'EOF' +SELECT 1; + ?column? +---------- + 1 +(1 row) + +EOF + + # Check that test target includes test-build as prerequisite + # test-build is .PHONY and should always appear when enabled + run make -n test 2>&1 + [ "$status" -eq 0 ] + echo "$output" | grep -q "test-build" +} + +@test "test-build runs independently of regular tests" { + # Add a new test file - builds on previous tests + cat > test/build/independent_test.sql <<'EOF' +SELECT 2; +EOF + + # Create expected output file matching pg_regress format for the new test + mkdir -p test/build/expected + cat > test/build/expected/independent_test.out <<'EOF' +SELECT 2; + ?column? +---------- + 2 +(1 row) + +EOF + + # Fix invalid_test.sql from test 5 - make it valid so test-build can pass + # This builds on the previous test by fixing the issue it introduced + cat > test/build/invalid_test.sql <<'EOF' +-- This SQL is now valid (fixed from previous test) +SELECT 3; +EOF + cat > test/build/expected/invalid_test.out <<'EOF' +-- This SQL is now valid (fixed from previous test) +SELECT 3; + ?column? +---------- + 3 +(1 row) + +EOF + + # Clean test/build/sql/ so updated files get copied (this is a generated directory) + rm -rf test/build/sql + + # Also ensure expected files exist for all other test/build files from previous tests + # disabled_test.sql - needs expected file + if [ -f "test/build/disabled_test.sql" ] && [ ! -f "test/build/expected/disabled_test.out" ]; then + cat > test/build/expected/disabled_test.out <<'EOF' +SELECT 1; + ?column? +---------- + 1 +(1 row) + +EOF + fi + + # test-build should run without needing regular test files (test/sql/) + run make test-build + [ "$status" -eq 0 ] +} + +@test "repository is still functional after test-build" { + # Basic sanity check + assert_file_exists "Makefile" + run make --version + [ "$status" -eq 0 ] +} + +# vi: expandtab sw=2 ts=2 + diff --git a/test/standard/test-test-install.bats b/test/standard/test-test-install.bats new file mode 100644 index 0000000..7a7e7f5 --- /dev/null +++ b/test/standard/test-test-install.bats @@ -0,0 +1,93 @@ +#!/usr/bin/env bats + +# Test: test/install feature +# +# Tests that the test/install feature works correctly: +# - Schedule files are generated when test/install/ has SQL files +# - Install schedule lists files with ../install/ prefix +# - test/install can be disabled via PGXNTOOL_ENABLE_TEST_INSTALL +# - make clean removes generated schedule files + +load ../lib/helpers + +setup_file() { + # Set TOPDIR + setup_topdir + + # Independent test - gets its own isolated environment with foundation TEST_REPO + load_test_env "test-install" + ensure_foundation "$TEST_DIR" +} + +setup() { + load_test_env "test-install" + cd "$TEST_REPO" + # Create test directories in setup so files exist when Make parses + mkdir -p test/install test/sql test/expected +} + +@test "test/install not enabled when test/install/ is empty" { + # Remove all SQL files from test/install + rm -f test/install/*.sql + + # Schedule files should not be generated + run make -n installcheck 2>&1 + assert_success + # Should NOT reference install schedule + ! echo "$output" | grep -q "install/schedule" +} + +@test "schedule files generated when test/install/ has SQL files" { + # Create a SQL file in test/install with its expected output + cat > test/install/setup.sql <<'EOF' +SELECT 1; +EOF + printf 'SELECT 1;\n ?column? \n----------\n 1\n(1 row)\n\n' > test/install/setup.out + + # Dry-run should reference schedule files + run make -n installcheck 2>&1 + assert_success + echo "$output" | grep -q "schedule" +} + +@test "install schedule lists files with ../install/ prefix" { + # setup.sql created in previous test + run make test/install/schedule + assert_success + assert_file_exists "test/install/schedule" + + # Schedule should reference install files with relative path + run grep "../install/setup" test/install/schedule + assert_success +} + +@test "test/install can be disabled via PGXNTOOL_ENABLE_TEST_INSTALL" { + # When disabled, schedule files should not be generated + run make -n installcheck PGXNTOOL_ENABLE_TEST_INSTALL=no 2>&1 + assert_success + # Should NOT reference install schedule + ! echo "$output" | grep -q "install/schedule" +} + +@test "make clean removes install schedule file" { + # Generate schedule file first + make test/install/schedule + + assert_file_exists "test/install/schedule" + + # Run make clean + run make clean + assert_success + + # Schedule file should be removed + assert_file_not_exists "test/install/schedule" +} + +@test "repository is still functional after test/install tests" { + # Basic sanity check + assert_file_exists "Makefile" + run make --version + assert_success +} + +# vi: expandtab sw=2 ts=2 diff --git a/test/standard/test-verify-results.bats b/test/standard/test-verify-results.bats new file mode 100644 index 0000000..8b7569b --- /dev/null +++ b/test/standard/test-verify-results.bats @@ -0,0 +1,166 @@ +#!/usr/bin/env bats + +# Test: verify-results feature +# +# Tests that the verify-results feature works correctly: +# - verify-results blocks make results when tests are failing +# - verify-results allows make results when tests are passing +# - verify-results can be disabled via PGXNTOOL_ENABLE_VERIFY_RESULTS +# - verify-results has no dependencies (doesn't run tests itself) + +load ../lib/helpers + +setup_file() { + # Set TOPDIR + setup_topdir + + # Independent test - gets its own isolated environment with foundation TEST_REPO + load_test_env "verify-results" + ensure_foundation "$TEST_DIR" +} + +setup() { + load_test_env "verify-results" + cd "$TEST_REPO" +} + +@test "verify-results target exists (pgTap is default)" { + # Since pgTap is the default in base.mk, verify-results should exist + run make -n verify-results 2>&1 + [ "$status" -eq 0 ] +} + +@test "verify-results succeeds when no test failures exist" { + # Ensure regression.diffs doesn't exist (tests haven't failed) + # TESTOUT defaults to TESTDIR which is "test", so regression.diffs is at test/regression.diffs + rm -f test/regression.diffs + + # verify-results should succeed + run make verify-results + [ "$status" -eq 0 ] +} + +@test "verify-results fails when regression.diffs exists" { + # Create a fake regression.diffs file to simulate test failures + # TESTOUT defaults to TESTDIR which is "test" + cat > test/regression.diffs <<'EOF' +*** /path/to/test/expected/test.out +--- /path/to/test/results/test.out +*************** +*** 1 **** +! expected +--- 1 ---- +! actual +EOF + + # verify-results should fail + run make verify-results + [ "$status" -ne 0 ] + echo "$output" | grep -qE "(ERROR|failing|Cannot run)" +} + +@test "verify-results provides clear error message" { + # Create regression.diffs + # TESTOUT defaults to TESTDIR which is "test" + echo "test diff" > test/regression.diffs + + # Run verify-results + run make verify-results + [ "$status" -ne 0 ] + + # Check for helpful error message + echo "$output" | grep -q "Tests are failing" + echo "$output" | grep -q "Cannot run 'make results'" + echo "$output" | grep -q "regression.diffs" +} + +@test "make results is blocked by verify-results when tests are failing" { + # Create regression.diffs to simulate failures + # TESTOUT defaults to TESTDIR which is "test" + echo "test failure" > test/regression.diffs + + # make results should fail due to verify-results + run make results + [ "$status" -ne 0 ] + echo "$output" | grep -qE "(ERROR|failing|Cannot run)" +} + +@test "make results succeeds when tests are passing" { + # Ensure no regression.diffs exists + # TESTOUT defaults to TESTDIR which is "test" + rm -f test/regression.diffs + + # First, establish baseline expected output if needed + if [ ! -f "test/expected/pgxntool-test.out" ] || [ ! -s "test/expected/pgxntool-test.out" ]; then + # Run make test to generate expected output + # Note: This may fail if tests aren't set up correctly, but that's OK - + # we're just trying to establish a baseline if one doesn't exist + make test || true + # Copy results to expected if test passed + # Note: pg_regress may create results in test/results/ subdirectory + # Files may not exist if test failed, so ignore copy errors + if [ ! -r "test/regression.diffs" ] && [ -d "test/results" ]; then + mkdir -p test/expected + cp test/results/*.out test/expected/ 2>/dev/null || true + fi + fi + + # Ensure no failures + rm -f test/regression.diffs + + # make results should succeed (verify-results passes, then test runs) + # Note: This may take a while as it runs full test suite + run make results + [ "$status" -eq 0 ] +} + +@test "verify-results can be disabled via PGXNTOOL_ENABLE_VERIFY_RESULTS" { + # Create regression.diffs + # TESTOUT defaults to TESTDIR which is "test" + echo "test failure" > test/regression.diffs + + # Disable verify-results and verify it's not in the list of available targets + run make list PGXNTOOL_ENABLE_VERIFY_RESULTS= + [ "$status" -eq 0 ] + echo "$output" | grep -qv "verify-results" +} + +@test "verify-results has no dependencies" { + # Check that verify-results target doesn't depend on test or installcheck + # by examining the make output + run make -n verify-results 2>&1 + + # Should not show test or installcheck being run + echo "$output" | grep -v "test:" > /dev/null || { + # If test appears, it might be in a comment or variable expansion + # The key is that verify-results itself doesn't RUN tests + [ "$status" -eq 0 ] + } +} + +@test "verify-results only checks file existence" { + # verify-results should be fast - it just checks if regression.diffs exists + # Create the file + # TESTOUT defaults to TESTDIR which is "test" + echo "dummy diff" > test/regression.diffs + + # Time the execution (should be very fast) + start_time=$(date +%s) + run make verify-results 2>&1 + end_time=$(date +%s) + elapsed=$((end_time - start_time)) + + # Should complete in under 2 seconds (just file check) + [ "$elapsed" -lt 2 ] + [ "$status" -ne 0 ] +} + +@test "repository is still functional after verify-results tests" { + # Basic sanity check + assert_file_exists "Makefile" + run make --version + [ "$status" -eq 0 ] +} + +# vi: expandtab sw=2 ts=2 +