Schema-invariant pgTAP assertion descriptions in test/core/functions.sql

[jnasbyupgrade]

pgTAP's auto-generated test descriptions schema-qualify via ncs() (e.g. "Function public.null_count(...) should return int"), which pins the committed expected-output file to whichever schema count_nulls happens to land in. Pass an explicit, schema-free description to every call instead - function_returns/isnt_strict/volatility_is/lives_ok/bag_eq/throws_ok all still resolve and execute against the real, %I-qualified, ncs()-derived name; only the visible description text drops the schema. This is a self-contained cleanup with no dependency on TEST_SCHEMA existing - it's what lets a later schema-switching mechanism reuse a single expected-output file across every schema the suite runs against, instead of needing one per schema value.

Regenerated test/expected/extension_tests.out via make results.

[coderabbitai]

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: Pro Plus

Run ID: 23f70f8e-6a6d-4f26-ad26-43d65d94c3f5

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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Claude finished @jnasbyupgrade's task in 2m 18s —— View job

---

Claude finished the review

Todo list

• Gather context (read PR diff, repo CLAUDE.md, changed files)
• Understand the request (code review of PR Schema-invariant pgTAP assertion descriptions in test/core/functions.sql #46)
• Review test/core/functions.sql changes
• Review test/expected/extension_tests.out changes
• Post review feedback

---

Summary

The change does exactly what the PR description says: every pgTAP assertion in test/core/functions.sql now gets an explicit, schema-free description string, while the actual assertion arguments (ncs()-qualified names, %I-quoted identifiers) are untouched. The regenerated test/expected/extension_tests.out only drops public. from the description text, confirming the assertions themselves didn't change behavior. The new exec_desc parameter threading through pg_temp.test_trigger_raw / pg_temp.test_trigger is correctly plumbed (both the named-arg call site in test_trigger and the positional call site in test__functionality pass all 6 params in the right order).

Finding

test/core/functions.sql:86 — description text says "int", but the actual checked type is "integer"

RETURN NEXT function_returns(
  ncs(), f_name, f_args
  , 'int'::regtype::text -- Sanitize type name
  , format('Function %s(%s) should return int', f_name, array_to_string(f_args, ','))
);

'int'::regtype::text resolves to 'integer' (confirmed by the old auto-generated description still visible in the diff: Function public.null_count(anyarray) should return integer). The new hardcoded description says should return int instead of should return integer — a wording regression introduced by this PR, not present before (the old pgTAP-generated description always tracked the real resolved type name automatically; this new literal string doesn't).

It's cosmetic — the assertion still checks the real return type via the 3rd positional arg, so nothing fails — but it's now a second place that would need to be kept in sync with the type name if it's ever 'trigger', 'integer', etc., which cuts against the "no functional risk from a docs-only cleanup" premise a reviewer would otherwise take for granted. Consider format('Function %s(%s) should return integer', ...) (or format('Function %s(%s) should return %s', f_name, array_to_string(f_args, ','), 'int'::regtype::text) to derive it from the same source of truth used for the check) to keep the description accurate and self-updating.

Fix this →

Other notes (no action needed)

• test/core/functions.sql:161 (lives_ok( c_command, c_command_desc )) and the errdesc/exec_desc split in test_trigger_raw correctly keep the executed SQL schema-qualified while the description stays schema-free — this is the right pattern and matches the PR's stated goal.
• The doc comments added at test/core/functions.sql:73-82 and :136-141 clearly explain why the description is decoupled from the executed statement, which will help whoever wires up the schema-switching mechanism mentioned in the PR description.
• Description strings hardcoding "IMMUTABLE"/"trigger" (lines 98, 108, 114, 120) are fine since those literal comparison values can't drift the way the int/integer one did.