Skip to content

Fix object_group.sql's pgTAP plan count - #32

Merged
jnasbyupgrade merged 1 commit into
Postgres-Extensions:masterfrom
jnasbyupgrade:fix-object-group-plan-count
Aug 10, 2026
Merged

Fix object_group.sql's pgTAP plan count#32
jnasbyupgrade merged 1 commit into
Postgres-Extensions:masterfrom
jnasbyupgrade:fix-object-group-plan-count

Conversation

@jnasbyupgrade

Copy link
Copy Markdown
Contributor

The "Drop tests" section in `test/sql/object_group.sql` runs 4 assertions (3 `throws_ok`, 1 `lives_ok`, at lines 128/134/140/146) but the hand-computed `plan()` math only counted `+3` for it, so `plan()` totaled 24 against the 25 tests that actually run.

Traced by matching each named section's planned subtotal against the actual list of 25 executed test names -- every other section already matched. `make verify-results` (the real pgtap-aware gate `#30` switches CI to, as opposed to plain `pg_regress` diffing) is what catches this; it was invisible before because the stale `24` was already baked into the committed `test/expected/object_group.out`, so a plain diff always matched regardless.

`test/expected/object_group.out` regenerated via `make results`, not hand-edited.

Co-Authored-By: Claude noreply@anthropic.com

The "Drop tests" section runs 4 assertions (3 throws_ok, 1 lives_ok at
lines 128/134/140/146) but the plan math only counted +3 for it. Traced
by matching the plan formula's per-section subtotals against the actual
list of 25 executed test names -- every other section's count was
already correct. verify-results (make's real pgtap-aware gate, as
opposed to plain pg_regress diffing) is what catches this; it was
invisible before because the stale count was already baked into the
committed expected-output file, so a plain diff always matched.

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

coderabbitai Bot commented Aug 10, 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: Pro Plus

Run ID: c61abc7b-01f6-48b6-ae2d-b05ea6505bb5

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

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

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

Claude finished @jnasbyupgrade's task in 1m 16s —— View job


Code Review: Fix object_group.sql's pgTAP plan count

  • Gather context (PR diff, related files)
  • Review test/sql/object_group.sql change
  • Review test/expected/object_group.out change
  • Verify plan count matches actual test count
  • Post final review

Verdict: Correct fix, LGTM.

I independently re-summed the plan() section subtotals against the actual assertions in test/sql/object_group.sql:

Section Planned Actual assertions
setup 1 1 (line 45)
__create 2 2 (lines 53, 62)
__get 2+2=4 4 (lines 70×2, 74, 78)
__object__add 1 1 (line 94, TODO block commented out)
object__getsert with group 3 3 (lines 104, 118, 122)
Drop tests 4 (was 3) 4 (lines 128, 134, 140, 146)
__object__remove 4 4 (lines 152, 162, 172, 180)
__remove 4+2=6 6 (lines 188×2, 192, 198, 206, 214)
Total 25 25

That matches the 25 ok lines now in test/expected/object_group.out, and the stale # Looks like you planned 24 tests but ran 25 diagnostic line is correctly gone since plan() and the actual count now agree.

Both the .sql and regenerated .out changes are minimal, consistent with each other, and correctly scoped — no unrelated changes. CI (.github/workflows/ci.yml) has already run and passed on this branch.

One minor, non-blocking observation: other test files (_object_v.sql, all.sql, base.sql, capture.sql, event_trigger.sql) also use hand-computed plan() math and could theoretically have the same class of bug, but verifying those is out of scope for this PR — the PR body appropriately scopes this fix to object_group.sql only, and per the PR description this was specifically found via make verify-results, the pgtap-aware CI gate from #30.

No changes requested.

@jnasbyupgrade
jnasbyupgrade merged commit f2260f2 into Postgres-Extensions:master Aug 10, 2026
11 checks passed
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