Repository navigation
Clarify that test/install only prepares the test environment #144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -166,7 +166,11 @@ cp test/build/results/<name>.out test/build/expected/<name>.out | |||||||||||||||||||
| ---- | ||||||||||||||||||||
|
|
||||||||||||||||||||
| === test/install | ||||||||||||||||||||
| Runs setup files before the main test suite within the same `pg_regress` invocation. This allows expensive one-time operations (like extension installation) to set up state that persists into the regular test files. | ||||||||||||||||||||
| `test/install` only prepares the environment for your real tests; its output is not a test result. If a file in it fails, the run aborts, but short of a failure nothing about its output is checked: it is not compared against expected output and not validated as TAP, so don't write tests there (put them in `test/sql`). | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
Comment on lines
168
to
+170
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overstated claim: "If a file in it fails, the run aborts" This reads as unconditional, but abort-on-failure only happens when Lines 212 to 214 in d2e9236
So this new sentence contradicts the existing WARNING in the same section. Consider something like "If a file in it fails and has The identical overstatement was also introduced in this PR's CLAUDE.md edit, immediately followed by the correct qualifier ("...whatever the value"): Lines 209 to 211 in d2e9236
Comment on lines
168
to
+170
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New guidance conflicts with the existing WARNING's suggestion This sentence says "don't write tests there (put them in Lines 212 to 214 in d2e9236
("...If you need actual output comparison, put that logic in Read together, the doc now gives contradictory guidance about whether pgTap assertions belong in |
||||||||||||||||||||
| Abort-on-failure relies on `ON_ERROR_STOP`: pgxntool's static check requires every `test/install/*.sql` file to include `test/pgxntool/psql.sql` (which sets it) or to `\set` or `\unset ON_ERROR_STOP` itself. An explicit setting means the author has chosen deliberately. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| It runs setup files before the main test suite within the same `pg_regress` invocation. This allows expensive one-time operations (like extension installation) to set up state that persists into the regular test files. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| **How it works:** | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -202,11 +206,11 @@ PGXNTOOL_ENABLE_TEST_INSTALL = yes # or no | |||||||||||||||||||
|
|
||||||||||||||||||||
| **Why this is useful:** | ||||||||||||||||||||
|
|
||||||||||||||||||||
| Without `test/install`, each test file typically needs to run `CREATE EXTENSION` in its setup, which adds overhead and doesn't allow validating the installation step separately. With `test/install`, setup runs once before all tests, and any state it creates (tables, extensions, etc.) is available to every subsequent test file. | ||||||||||||||||||||
| Without `test/install`, each test file typically needs to run `CREATE EXTENSION` in its setup, which adds overhead and repeats the installation step in every test. With `test/install`, setup runs once before all tests, and any state it creates (tables, extensions, etc.) is available to every subsequent test file. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| **Key detail:** Install files and regular tests run in a single `pg_regress` invocation. This means the database is NOT dropped between install and test phases — state created by install files persists into the main test suite. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| WARNING: **`+test/install/*.out+` is never actually compared against anything.** Unlike every other test type pgxntool supports, the `test/install` directory's actual output is written to the exact same file as its expected output, so there is no diff — a wrong or changed result will never fail the build, no matter how much it changes. The *only* thing that still fails the build is a hard SQL error, and only if the file sets `ON_ERROR_STOP` (directly, or via `\i test/pgxntool/psql.sql`) — without it, psql prints the error, keeps going, and the file "passes" regardless. pgxntool enforces this by default (`_check-test-install-error-stop`, see `PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK` to disable), but that only guarantees errors are caught — it does not give you real output validation. If you need actual output comparison, put that logic in `test/sql` instead (or use pgTap assertions from within the install file itself). | ||||||||||||||||||||
| WARNING: **`+test/install/*.out+` is not a test result and is never compared against anything.** Unlike every other test type pgxntool supports, the `test/install` directory's actual output is written to the exact same file as its expected output, so there is no diff — a wrong or changed result will never fail the build, no matter how much it changes. The *only* thing that still fails the build is a hard SQL error, and only if the file sets `ON_ERROR_STOP` (directly, or via `\i test/pgxntool/psql.sql`) — without it, psql prints the error, keeps going, and the file "passes" regardless. pgxntool enforces this by default (`_check-test-install-error-stop`, see `PGXNTOOL_ENABLE_TEST_INSTALL_ERROR_STOP_CHECK` to disable), but that only guarantees errors are caught — it does not give you real output validation. If you need actual output comparison, put that logic in `test/sql` instead (or use pgTap assertions from within the install file itself). | ||||||||||||||||||||
|
|
||||||||||||||||||||
| ==== Update & Upgrade (U&U) Testing | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same overstatement as in the README.asc edit
"but a failure in it must abort the run" is stated unconditionally, but the very next clause (unchanged by this PR) says the check accepts a file with
\set/\unset ON_ERROR_STOP"whatever the value" — i.e. explicitly turning it off also passes the check, and in that case a failure would not abort the run. Same issue as the README.asc edit in this PR:pgxntool/README.asc
Lines 168 to 170 in d2e9236
Consider qualifying this, e.g. "...but a failure in it must abort the run if
ON_ERROR_STOPis on."