Skip to content

Fix make test failing when test/sql/ does not exist - #133

Merged
jnasbyupgrade merged 1 commit into
Postgres-Extensions:masterfrom
jnasbyupgrade:pre-2.4.0-testsql-dir-rule
Oct 7, 2026
Merged

jnasbyupgrade merged 1 commit into
Postgres-Extensions:masterfrom
jnasbyupgrade:pre-2.4.0-testsql-dir-rule

Conversation

@jnasbyupgrade

@jnasbyupgrade jnasbyupgrade commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

make test failed with No rule to make target 'test/sql/', needed by 'installcheck' in a project with no test/sql/ directory. The rule that creates the directory was spelled $(TESTDIR)/sql while installcheck's order-only prerequisite is $(TESTDIR)/sql/, and make treats those as different targets. The rule now targets $(TESTDIR)/sql/, matching its expected/ and results/ siblings; test-build is fixed too, since it runs installcheck recursively. setup.sh normally creates test/sql/, which hid this.

Merge this first, then Postgres-Extensions/pgxntool-test#100, which tests it.

HISTORY.asc gets a STABLE entry (no issue, so no Fixes line).

Audit

Every spelling of these directories in base.mk and the .mk/.sh scripts was checked. This rule target was the only mismatch. The other slash-less spellings are shell commands in recipes (mkdir -p), variables (TEST_BUILD_SQL_DIR, EXTRA_CLEAN), or script-created directories, not make targets. README.asc and CLAUDE.md mention test/sql/ only as where tests live; nothing documents the missing-directory case.

Testing

pgxntool-test#100 adds a make -n installcheck TESTDIR=<nonexistent> check to test/standard/base-mk-misc.bats. It fails without this fix; with it, the full suite passes, 316/316, 0 skipped.

🤖 Generated with Claude Code

`installcheck` has an order-only prerequisite on `$(TESTDIR)/sql/`, but the
rule that creates it was spelled `$(TESTDIR)/sql`. Make treats the two as
different targets, so a project without a test/sql/ directory failed with
"No rule to make target 'test/sql/', needed by 'installcheck'". setup.sh
normally creates the directory, which hid this. The rule now uses the same
trailing-slash spelling as the prerequisite and as its `test/expected/` and
`results/` siblings.

Related changes in pgxntool-test:
- Add a `make -n installcheck` check against a TESTDIR with no sql/,
  expected/ or results/ directory

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2236a271-a29f-4200-9d46-10cc2c43709c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@jnasbyupgrade
jnasbyupgrade merged commit 51062d3 into Postgres-Extensions:master Oct 7, 2026
12 of 13 checks passed
@jnasbyupgrade
jnasbyupgrade deleted the pre-2.4.0-testsql-dir-rule branch October 7, 2026 21:50
jnasbyupgrade added a commit to Postgres-Extensions/pgxntool-test that referenced this pull request Oct 7, 2026
Adds coverage for
Postgres-Extensions/pgxntool#133. That PR fixes
`make test` failing with `No rule to make target 'test/sql/', needed by
'installcheck'` in a project with no `test/sql/` directory.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant