From 7cc3cda720bbc462aad1b54298ac35ee1d171f86 Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 13:09:14 -0400 Subject: [PATCH 01/10] initial support for `pg_regress`-based tests --- Cargo.lock | 25 + cargo-pgrx/Cargo.toml | 1 + cargo-pgrx/README.md | 222 ++++++- cargo-pgrx/src/command/connect.rs | 2 +- cargo-pgrx/src/command/mod.rs | 1 + cargo-pgrx/src/command/new.rs | 45 +- cargo-pgrx/src/command/pgrx.rs | 2 + cargo-pgrx/src/command/regress.rs | 545 ++++++++++++++++++ cargo-pgrx/src/command/run.rs | 64 +- cargo-pgrx/src/command/start.rs | 51 +- cargo-pgrx/src/manifest.rs | 4 +- cargo-pgrx/src/templates/gitignore | 3 + cargo-pgrx/src/templates/setup_out | 3 + cargo-pgrx/src/templates/setup_sql | 3 + pgrx-examples/range/.gitignore | 3 + .../range/pg_regress/expected/make_range.out | 6 + .../range/pg_regress/expected/setup.out | 3 + .../pg_regress/expected/store_ranges.out | 116 ++++ .../range/pg_regress/sql/make_range.sql | 1 + pgrx-examples/range/pg_regress/sql/setup.sql | 3 + .../range/pg_regress/sql/store_ranges.sql | 12 + pgrx-pg-config/src/lib.rs | 77 ++- 22 files changed, 1149 insertions(+), 43 deletions(-) create mode 100644 cargo-pgrx/src/command/regress.rs create mode 100644 cargo-pgrx/src/templates/setup_out create mode 100644 cargo-pgrx/src/templates/setup_sql create mode 100644 pgrx-examples/range/pg_regress/expected/make_range.out create mode 100644 pgrx-examples/range/pg_regress/expected/setup.out create mode 100644 pgrx-examples/range/pg_regress/expected/store_ranges.out create mode 100644 pgrx-examples/range/pg_regress/sql/make_range.sql create mode 100644 pgrx-examples/range/pg_regress/sql/setup.sql create mode 100644 pgrx-examples/range/pg_regress/sql/store_ranges.sql diff --git a/Cargo.lock b/Cargo.lock index 6f4c4439b1..283c6792d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -414,6 +414,7 @@ dependencies = [ "tracing-subscriber", "ureq", "url", + "which", "zip-extract", ] @@ -858,6 +859,12 @@ dependencies = [ "syn", ] +[[package]] +name = "env_home" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7f84e12ccf0a7ddc17a6c41c93326024c42920d7ee630d04950e6926645c0fe" + [[package]] name = "env_proxy" version = "0.4.1" @@ -3694,6 +3701,18 @@ dependencies = [ "rustls-pki-types", ] +[[package]] +name = "which" +version = "7.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24d643ce3fd3e5b54854602a080f34fb10ab75e0b813ee32d00ca2b44fa74762" +dependencies = [ + "either", + "env_home", + "rustix", + "winsafe", +] + [[package]] name = "whoami" version = "1.6.0" @@ -3979,6 +3998,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "winsafe" +version = "0.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d135d17ab770252ad95e9a872d365cf3090e3be864a34ab46f48555993efc904" + [[package]] name = "wit-bindgen-rt" version = "0.39.0" diff --git a/cargo-pgrx/Cargo.toml b/cargo-pgrx/Cargo.toml index 0dae75fa28..be1478106b 100644 --- a/cargo-pgrx/Cargo.toml +++ b/cargo-pgrx/Cargo.toml @@ -57,6 +57,7 @@ serde-xml-rs = "0.6.0" tar = "0.4.44" ureq = { version = "3.0.10", default-features = false, features = ["gzip"] } url.workspace = true +which = "7.0.3" zip-extract = "0.2.2" # SQL schema generation diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index 962d7fed21..1bfdf25748 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -451,7 +451,7 @@ Options: -V, --version Print version ``` -## Testing Your Extension +## Testing with Unit Tests ```console $ cargo pgrx test @@ -525,6 +525,226 @@ Options: -V, --version Print version ``` +## Testing with Regression Tests + +pgrx supports a regression test system very similar to the one prescribed by Postgres' `pg_regress` tool. In fact, pgrx uses `pg_regress` to run the regression tests. + +`cargo pgrx regress` is used to run the regression tests. It has a number of options similar to `cargo pgrx test`: + +```console +$ cargo pgrx regress --help +Run the regression test suite for this crate + +Usage: cargo pgrx regress [OPTIONS] [PG_VERSION] [TEST_FILTER] + +Arguments: + [PG_VERSION] Do you want to run against pg13, pg14, pg15, pg16, pg17? [env: PG_VERSION=] + [TEST_FILTER] If specified, only run tests containing this string in their names + +Options: + --dbname If specified, use this database name instead of the auto-generated version of `$extname_regress` + --resetdb Recreate the test database, even if it already exists + -v, --verbose... Enable info logs, -vv for debug, -vvv for trace + -p, --package Package to build (see `cargo help pkgid`) + --manifest-path Path to Cargo.toml + -r, --release compile for release mode (default is debug) + --profile Specific profile to use (conflicts with `--release`) + -n, --no-schema Don't regenerate the schema + --runas Use `sudo` to initialize and run the Postgres test instance as this system user + --pgdata Initialize the test database cluster here, instead of the default location. If used with `--runas`, then it must be writable by the user + --all-features Activate all available features + --no-default-features Do not activate the `default` feature + -F, --features Space-separated list of features to activate + --postgresql-conf Custom `postgresql.conf` settings in the form of `key=value`, ie `log_min_messages=debug1` + -h, --help Print help + -V, --version Print version +``` + +Regression tests are split into `*.sql` files and `*.out` files. The files themselves are organized into separate directories rooted at `./pg_regress`. + +For example, using our [range example](../pgrx-examples/range/), the directory structure looks like this: + +```console +$ tree +. +├── Cargo.lock +├── Cargo.toml +├── README.md +├── pg_regress +│ ├── sql # these are the corresponding test output files +│ │ ├── make_range.sql +│ │ ├── setup.sql +│ │ └── store_ranges.sql +│ └── expected # these are the individual regression test scripts +│ ├── make_range.out +│ ├── setup.out +│ └── store_ranges.out +├── range.control +├── results +└── src + ├── bin + │ └── pgrx_embed.rs + └── lib.rs +``` + +`setup.sql` is a special test in that it's run first, by itself, whenever the test database is first created, or reset using the `--resetdb` argument. + +When creating a new test, first make the `.sql` file in `./pg_regress/sql/` and then run `cargo pgrx regress`. pgrx will detect that the file is new and interactively prompt you to add its output, automatically adding it to git (if the directory is managed by git). + +For example, + +```console +$ echo "SELECT 1;" > ./pg_regress/sql/example.sql +$ cargo pgrx regress + Using DefaultFeature("pg13") and `pg_config` from ~/.pgrx/13.20/pgrx-install/bin/pg_config + Stopping Postgres v13 + Building extension with features pg13 + Running command " ~/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo" "build" "--lib" "--features" "pg13" "--no-default-features" "--message-format=json-render-diagnostics" + Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s + Installing extension + Copying control file to ~/.pgrx/13.20/pgrx-install/share/postgresql/extension/range.control + Copying shared library to ~/.pgrx/13.20/pgrx-install/lib/postgresql/range.so + Discovered 9 SQL entities: 0 schemas (0 unique), 9 functions, 0 types, 0 enums, 0 sqls, 0 ords, 0 hashes, 0 aggregates, 0 triggers + Rebuilding pgrx_embed, in debug mode, for SQL generation with features pg13 + Compiling range v0.0.0 ( ~/_work/pgrx/pgrx-examples/range) + Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.70s + Writing SQL entities to ~/.pgrx/13.20/pgrx-install/share/postgresql/extension/range--0.1.0.sql + Finished installing range + Starting Postgres v13 on port 28813 + Re-using existing database range_regress + Found 1 new tests, running each individually to create output + Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "example" "--launcher=/tmp/pgrx-pg_regress-runner-2940893.sh" +----------- +SELECT 1; + ?column? +---------- + 1 +(1 row) + + +test `example` generated the above output: +Accept [Y, n]? +``` + +Typing `Y` (or just pressing return) will copy the test output to the proper location, `./pg_regress/expected/example.sql` and then run the entire test suite: + +```console +... +test `example` generated the above output: +Accept [Y, n]? y + Copying test output to ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/example.out + Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "example" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2940893.sh" +(using postmaster on localhost, port 28813) +============== running regression test queries ============== +test example ... ok 6 ms +test make_range ... ok 18 ms +test store_ranges ... ok 20 ms + +===================== + All 3 tests passed. +===================== +``` + +### Things to Know + +- `setup.sql` is only executed when tests are run for the first time, or the `--resetdb` argument is used +- The point of `setup.sql` is to perform some heavy-weight database object creation/data-loading _only_ when the test regression database is created. +- tests are executed in alphabetical order +- pgrx creates a database named `$extname_regress` unless `--dbname` is used +- Postgres' documentation for `pg_regress` [begins here](https://www.postgresql.org/docs/current/regress.html). While pgrx does not support every knob and dial, its organization is largely compatible (PRs welcome to enhance features) +- to regenerate the expected test output, delete the `./pg_regress/expected/TEST_NAME.out` file and run `cargo pgrx regress`. You'll be prompted to accept the new output and it'll automatically be run through `git add` +- `pg_regress` uses `psql` to run each test and literally diffs the output against the expected output file. pgrx does two things to help eliminate noise in the test output. The first is it sets `client_min_messages=warning` when starting the Postgres instance and it also passes `-v VERBOSITY=terse` through to `psql`. +- when test output includes results from a table (ie, a SELECT statement), watch out for ordering differences between runs. Prefer to always "ORDER BY ..." any such statements. + +### Be Kind to Yourself + +While always good advice, in the context of individual regression tests, this means that a test should not leave anything behind or should be able to tolerate leftover database objects from previous runs. + +Each test should be written to "DROP IF EXISTS ... ; CREATE ..." or instead "DROP ..." every database object at the end of the test. + +You can ignore this advice if you **always** run the regression tests with `--resetdb` as you'll have a clean database each time + +An example of a poorly designed test would be: + +```console +$ echo "CREATE TABLE foo();" > ./pg_regress/sql/bad.sql +$ cargo pgrx regress + Using DefaultFeature("pg13") and `pg_config` from ~/.pgrx/13.20/pgrx-install/bin/pg_config + Stopping Postgres v13 + Building extension with features pg13 + Running command " ~/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo" "build" "--lib" "--features" "pg13" "--no-default-features" "--message-format=json-render-diagnostics" + Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.04s + Installing extension + Copying control file to ~/.pgrx/13.20/pgrx-install/share/postgresql/extension/range.control + Copying shared library to ~/.pgrx/13.20/pgrx-install/lib/postgresql/range.so + Discovered 9 SQL entities: 0 schemas (0 unique), 9 functions, 0 types, 0 enums, 0 sqls, 0 ords, 0 hashes, 0 aggregates, 0 triggers + Rebuilding pgrx_embed, in debug mode, for SQL generation with features pg13 + Compiling range v0.0.0 ( ~/_work/pgrx/pgrx-examples/range) + Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.71s + Writing SQL entities to ~/.pgrx/13.20/pgrx-install/share/postgresql/extension/range--0.1.0.sql + Finished installing range + Starting Postgres v13 on port 28813 + Re-using existing database range_regress + Found 1 new tests, running each individually to create output + Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "bad" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" +----------- +CREATE TABLE foo(); + +test `bad` generated the above output: +Accept [Y, n]? +``` + +That looks like the perfect output, so we accept it: + +```console +... +test `bad` generated the above output: +Accept [Y, n]? Y + Copying test output to ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/bad.out + Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "bad" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" +(using postmaster on localhost, port 28813) +============== running regression test queries ============== +test bad ... FAILED 5 ms +test make_range ... ok 18 ms +test store_ranges ... ok 19 ms + +====================== + 1 of 3 tests failed. +====================== + +The differences that caused some tests to fail can be viewed in the +file " ~/_work/pgrx/pgrx-examples/range/pg_regress/regression.diffs". A copy of the test summary that you see +above is saved in the file " ~/_work/pgrx/pgrx-examples/range/pg_regress/regression.out". +``` + +And you see the `bad` test immediately failed! To see how it failed, look at the `./pg_regress/regression.diffs` file: + +```console +$ diff -U3 ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/bad.out ~/_work/pgrx/pgrx-examples/range/pg_regress/results/bad.out +--- ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/bad.out 2025-05-07 12:15:10.759010127 -0400 ++++ ~/_work/pgrx/pgrx-examples/range/pg_regress/results/bad.out 2025-05-07 12:15:10.775009912 -0400 +@@ -1 +1,2 @@ + CREATE TABLE foo(); ++ERROR: relation "foo" already exists +``` + +What you wanted in the .sql file is either + +```sql +CREATE TABLE IF NOT EXISTS foo(); +``` + +or + +```sql +CREATE TABLE foo(); +-- ... +DROP TABLE foo; +``` + +Either be resilient in the face of existing objects or cleanup when the test is finished. + + ## Building an Installation Package ```console diff --git a/cargo-pgrx/src/command/connect.rs b/cargo-pgrx/src/command/connect.rs index ebb580e910..a4d0be8663 100644 --- a/cargo-pgrx/src/command/connect.rs +++ b/cargo-pgrx/src/command/connect.rs @@ -89,7 +89,7 @@ impl CommandExecute for Connect { ))] pub(crate) fn connect_psql(pg_config: &PgConfig, dbname: &str, pgcli: bool) -> eyre::Result<()> { // restart postgres - start_postgres(pg_config)?; + start_postgres(pg_config, &Default::default())?; // create the named database if !createdb(pg_config, dbname, false, true, None)? { diff --git a/cargo-pgrx/src/command/mod.rs b/cargo-pgrx/src/command/mod.rs index 1b661b8e00..1f062f674a 100644 --- a/cargo-pgrx/src/command/mod.rs +++ b/cargo-pgrx/src/command/mod.rs @@ -19,6 +19,7 @@ pub(crate) mod install; pub(crate) mod new; pub(crate) mod package; pub(crate) mod pgrx; +mod regress; pub(crate) mod run; pub(crate) mod schema; pub(crate) mod start; diff --git a/cargo-pgrx/src/command/new.rs b/cargo-pgrx/src/command/new.rs index 6b39e75c92..445400ee9c 100644 --- a/cargo-pgrx/src/command/new.rs +++ b/cargo-pgrx/src/command/new.rs @@ -57,28 +57,19 @@ pub(crate) fn create_crate_template( create_dotcargo_config_toml(path.clone(), name)?; create_lib_rs(path.clone(), name, is_bgworker)?; create_git_ignore(path.clone(), name)?; - create_pgrx_embed_rs(path)?; + create_pgrx_embed_rs(path.clone())?; + create_setup_sql(path.clone(), name)?; + create_setup_out(path.clone(), name)?; Ok(()) } -fn create_directory_structure(mut src_dir: PathBuf) -> Result<(), std::io::Error> { - src_dir.push("src"); - std::fs::create_dir_all(&src_dir)?; - - src_dir.push("bin"); - std::fs::create_dir_all(&src_dir)?; - src_dir.pop(); - - src_dir.pop(); - - src_dir.push(".cargo"); - std::fs::create_dir_all(&src_dir)?; - src_dir.pop(); - - src_dir.push("sql"); - std::fs::create_dir_all(&src_dir)?; - src_dir.pop(); +fn create_directory_structure(root: PathBuf) -> Result<(), std::io::Error> { + std::fs::create_dir_all(root.join(".cargo"))?; + std::fs::create_dir_all(root.join("src").join("bin"))?; + std::fs::create_dir_all(root.join("pg_regress").join("expected"))?; + std::fs::create_dir_all(root.join("pg_regress").join("sql"))?; + std::fs::create_dir_all(root.join("sql"))?; Ok(()) } @@ -148,3 +139,21 @@ fn create_pgrx_embed_rs(mut filename: PathBuf) -> Result<(), std::io::Error> { file.write_all(include_bytes!("../templates/pgrx_embed_rs"))?; Ok(()) } + +fn create_setup_sql(mut filename: PathBuf, name: &str) -> Result<(), std::io::Error> { + filename.push("pg_regress"); + filename.push("sql"); + filename.push("setup.sql"); + let mut file = std::fs::File::create(filename)?; + file.write_all(format!(include_str!("../templates/setup_sql"), name = name).as_bytes())?; + Ok(()) +} + +fn create_setup_out(mut filename: PathBuf, name: &str) -> Result<(), std::io::Error> { + filename.push("pg_regress"); + filename.push("expected"); + filename.push("setup.out"); + let mut file = std::fs::File::create(filename)?; + file.write_all(format!(include_str!("../templates/setup_out"), name = name).as_bytes())?; + Ok(()) +} diff --git a/cargo-pgrx/src/command/pgrx.rs b/cargo-pgrx/src/command/pgrx.rs index 5c016fed20..d1a2333b96 100644 --- a/cargo-pgrx/src/command/pgrx.rs +++ b/cargo-pgrx/src/command/pgrx.rs @@ -43,6 +43,7 @@ enum CargoPgrxSubCommands { Get(super::get::Get), Cross(super::cross::Cross), Upgrade(super::upgrade::Upgrade), + Regress(super::regress::Regress), } impl CommandExecute for CargoPgrxSubCommands { @@ -65,6 +66,7 @@ impl CommandExecute for CargoPgrxSubCommands { Get(c) => c.execute(), Cross(c) => c.execute(), Upgrade(c) => c.execute(), + Regress(c) => c.execute(), } } } diff --git a/cargo-pgrx/src/command/regress.rs b/cargo-pgrx/src/command/regress.rs new file mode 100644 index 0000000000..b041e5daf6 --- /dev/null +++ b/cargo-pgrx/src/command/regress.rs @@ -0,0 +1,545 @@ +//LICENSE Portions Copyright 2019-2021 ZomboDB, LLC. +//LICENSE +//LICENSE Portions Copyright 2021-2023 Technology Concepts & Design, Inc. +//LICENSE +//LICENSE Portions Copyright 2023-2023 PgCentral Foundation, Inc. +//LICENSE +//LICENSE All rights reserved. +//LICENSE +//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. +use crate::command::get::get_property; +use crate::command::run::Run; +use crate::command::start::collect_postgresql_conf_settings; +use crate::manifest::get_package_manifest; +use crate::CommandExecute; +use owo_colors::OwoColorize; +use pgrx_pg_config::{createdb, dropdb, PgConfig}; +use std::collections::HashSet; +use std::env::temp_dir; +use std::fs::{DirEntry, File}; +use std::io::{IsTerminal, Write}; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitStatus, Stdio}; + +/// Run the regression test suite for this crate +#[derive(clap::Args, Debug, Clone)] +#[clap(author)] +pub(crate) struct Regress { + /// Do you want to run against pg13, pg14, pg15, pg16, pg17? + #[clap(env = "PG_VERSION")] + pub(crate) pg_version: Option, + /// If specified, only run tests containing this string in their names + pub(crate) test_filter: Option, + + /// If specified, use this database name instead of the auto-generated version of `$extname_regress` + #[clap(long)] + pub(crate) dbname: Option, + + /// Recreate the test database, even if it already exists + #[clap(long)] + pub(crate) resetdb: bool, + /// Package to build (see `cargo help pkgid`) + #[clap(long, short)] + pub(crate) package: Option, + /// Path to Cargo.toml + #[clap(long, value_parser)] + pub(crate) manifest_path: Option, + /// compile for release mode (default is debug) + #[clap(long, short)] + pub(crate) release: bool, + /// Specific profile to use (conflicts with `--release`) + #[clap(long)] + pub(crate) profile: Option, + /// Don't regenerate the schema + #[clap(long, short)] + pub(crate) no_schema: bool, + /// Use `sudo` to initialize and run the Postgres test instance as this system user + #[clap(long, value_name = "USER")] + pub(crate) runas: Option, + /// Initialize the test database cluster here, instead of the default location. If used with `--runas`, then it must be writable by the user + #[clap(long, value_name = "DIR")] + pub(crate) pgdata: Option, + #[clap(flatten)] + pub(crate) features: clap_cargo::Features, + #[clap(from_global, action = clap::ArgAction::Count)] + pub(crate) verbose: u8, + + /// Custom `postgresql.conf` settings in the form of `key=value`, ie `log_min_messages=debug1` + #[clap(long)] + pub(crate) postgresql_conf: Vec, +} + +impl Regress { + fn list_sql_tests( + &self, + manifest_path: impl AsRef, + ) -> eyre::Result<(Vec, Option)> { + let sql = manifest_path_to_sql_tests_path(manifest_path); + if !sql.exists() { + std::fs::create_dir(&sql)?; + } + let mut files = std::fs::read_dir(sql)?.collect::, _>>()?; + + let setup_file = Self::organize_files(&mut files, "sql"); + Ok((files, setup_file)) + } + + fn list_expected_outputs( + &self, + manifest_path: impl AsRef, + ) -> eyre::Result<(Vec, Option)> { + let expected = manifest_path_to_expected_tests_output_path(manifest_path); + if !expected.exists() { + std::fs::create_dir(&expected)?; + } + let mut files = std::fs::read_dir(expected)?.collect::, _>>()?; + + let setup_file = Self::organize_files(&mut files, "out"); + + Ok((files, setup_file)) + } + + fn organize_files(files: &mut Vec, only: &str) -> Option { + // remove any files that don't have `only` as the extension + files.retain(|entry| { + entry + .metadata() + .map(|metadata| { + metadata.is_file() + && entry + .file_name() + .to_str() + .map(|filename| filename.ends_with(&format!(".{only}"))) + .unwrap_or_default() + }) + .unwrap_or_default() + }); + + // `setup.{only}` is a special file that we handle separately + let is_setup = |entry: &DirEntry| { + if let Some(filename) = entry.file_name().to_str() { + if filename.ends_with(&format!("setup.{only}")) { + return true; + } + } + false + }; + + let setup_entry = + files.iter().position(|entry| is_setup(entry)).map(|idx| files.remove(idx)); + + // not all filesystems list directories sorted and we want some kind of guaranteed evaluation order + files.sort_unstable_by_key(|entry| entry.file_name()); + + setup_entry + } + + fn accept_new_test( + &self, + manifest_path: impl AsRef, + test_result_output: impl AsRef, + ) -> eyre::Result<()> { + if !std::io::stdin().is_terminal() { + panic!("not a terminal: cannot perform user interaction to accept tests") + } + let test_name = test_result_output + .as_ref() + .file_stem() + .expect("test result output should have a stem") + .to_str() + .expect("test result output filename should be valid UTF8") + .to_string(); + let test_output = std::fs::read_to_string(&test_result_output)?; + + println!("-----------"); + println!("{}", test_output.white()); + println!("test `{}` generated the above output:", test_name.bold().green()); + eprint!("Accept [Y, n]? "); + + let mut user_input = String::new(); + std::io::stdin().read_line(&mut user_input)?; + let user_input = user_input.trim(); + + let variant_suffix: Option; + if user_input == "Y" || user_input == "y" { + variant_suffix = None + } else if user_input.as_bytes()[0] >= b'0' && user_input.as_bytes()[0] <= b'9' { + // currently secret options to create a variant file + // however, postgres requires the original `test_name.out` to also exist + variant_suffix = Some(format!("_{user_input}")); + } else { + std::process::exit(1); + } + + let expected_path = manifest_path_to_expected_tests_output_path(manifest_path) + .join(&format!("{test_name}{}.out", variant_suffix.unwrap_or_default())); + + println!( + "{} test output to {}", + " Copying".bold().green(), + expected_path.display().bold().cyan() + ); + std::fs::copy(test_result_output, &expected_path)?; + + add_to_git(expected_path) + } + + fn run_all_tests( + &self, + pg_config: &PgConfig, + manifest_path: impl AsRef, + pgregress_path: impl AsRef, + dbname: &str, + test_files: &[&DirEntry], + output_files: &[&DirEntry], + ) -> eyre::Result<()> { + let test_names = test_files.iter().map(|e| make_test_name(*e)).collect::>(); + let output_names = output_files.iter().map(|e| make_test_name(*e)).collect::>(); + + if test_names.len() > output_names.len() { + // there are more tests than there are expected outputs, so figure out which ones + // don't have outputs and run them individually to create their outputs + + let new_tests = test_files + .iter() + .filter(|entry| { + let test_name = make_test_name(entry); + !output_names.contains(&test_name) + }) + .collect::>(); + + println!( + "{} {} new tests, running each individually to create output", + " Found".bold().cyan(), + new_tests.len() + ); + for new_test in new_tests { + if let Some(test_result_output) = create_regress_output( + &pg_config, + &manifest_path, + &pgregress_path, + &dbname, + new_test, + )? { + self.accept_new_test(&manifest_path, test_result_output)?; + } + } + } + + // now that all tests have outputs, run them all + run_tests(pg_config, pgregress_path, dbname, test_files) + } +} + +impl CommandExecute for Regress { + #[tracing::instrument(level = "error", skip(self))] + fn execute(mut self) -> eyre::Result<()> { + let (_, manifest_path) = get_package_manifest( + &self.features, + self.package.as_ref(), + self.manifest_path.as_ref(), + )?; + let extname = get_property(&manifest_path, "extname")? + .expect("extension name property `extname` should always be known"); + self.dbname = Some(self.dbname.unwrap_or_else(|| format!("{extname}_regress"))); + + // we purposely want as little noise as possible to end up in the expected test output files + self.postgresql_conf.push("client_min_messages=warning".into()); + let postgresql_conf = collect_postgresql_conf_settings(&self.postgresql_conf); + + // install the extension + let (pg_config, dbname) = Run::from(&self).install(false, &postgresql_conf)?; + let pgregress_path = pg_config.pg_regress_path()?; + + // figure out what test and output files we have + let (mut test_files, setup_file) = self.list_sql_tests(&manifest_path)?; + let (output_files, setup_output) = self.list_expected_outputs(&manifest_path)?; + + // NB: the `is_test` argument for both `dropdb()` and `createdb()` is for `cargo pgrx test`, + // which creates its own Postgres instance and has its own port and datadir and such, so we + // say `false` here. + if self.resetdb { + dropdb(&pg_config, &dbname, false, self.runas.clone())?; + } + // won't re-create it if it already exists + let created_db = createdb(&pg_config, &dbname, false, true, self.runas.clone())?; + if !created_db { + println!("{} existing database {dbname}", " Re-using".bold().cyan()); + } else { + match (setup_file, setup_output) { + // there is no setup test + (None, _) => {} + + // run the setup test, comparing its result to its output + (Some(setup_file), Some(_)) => { + run_tests(&pg_config, &pgregress_path, &dbname, &[&setup_file])? + } + + // create the output for the setup test + (Some(setup_file), None) => { + if let Some(test_result_output) = create_regress_output( + &pg_config, + &manifest_path, + &pgregress_path, + &dbname, + &setup_file, + )? { + // and ask the user if it's good + self.accept_new_test(&manifest_path, test_result_output)?; + } + } + } + } + + // filter tests + if let Some(test_filter) = self.test_filter.as_ref() { + test_files.retain(|entry| make_test_name(entry).contains(test_filter)); + if test_files.is_empty() { + println!( + "{} no tests matching filter `{test_filter}`", + " ERROR".bold().red() + ); + std::process::exit(1); + } + } + + self.run_all_tests( + &pg_config, + &manifest_path, + &pgregress_path, + &dbname, + &test_files.iter().collect::>(), + &output_files.iter().collect::>(), + ) + } +} + +fn run_tests( + pg_config: &PgConfig, + pg_regress_bin: impl AsRef, + dbname: &str, + test_files: &[&DirEntry], +) -> eyre::Result<()> { + if test_files.is_empty() { + return Ok(()); + } + let input_dir = test_files[0].path(); + let input_dir = input_dir + .parent() + .expect("test file should not be at the root of the filesystem") + .parent() + .expect("test file should be in a directory named `sql/`") + .to_path_buf(); + let (status, output) = pg_regress(pg_config, pg_regress_bin, dbname, &input_dir, test_files)?; + + println!("{output}"); + + if !status.success() { + std::process::exit(status.code().unwrap_or(1)); + } + + Ok(()) +} + +fn create_regress_output( + pg_config: &PgConfig, + manifest_path: impl AsRef, + pg_regress_bin: impl AsRef, + dbname: &str, + test_file: &DirEntry, +) -> eyre::Result> { + let test_name = make_test_name(&test_file); + let input_dir = test_file.path(); + let input_dir = input_dir + .parent() + .expect("test file should not be at the root of the filesystem") + .parent() + .expect("test file should be in a directory named `sql/`") + .to_path_buf(); + let (status, output) = pg_regress(pg_config, pg_regress_bin, dbname, &input_dir, &[test_file])?; + + if !status.success() { + // pg_regress returned with an error code, but that is most likely because the test's output file + // doesn't exist, since we are creating the test output. So if that's the case, if we have + // a `.out` file for it in the results/ directory, then we're successful + let out_file = + manifest_path_to_results_output_path(&manifest_path).join(&format!("{test_name}.out")); + if out_file.exists() { + return Ok(Some(out_file)); + } else { + println!("{output}"); + std::process::exit(status.code().unwrap_or(1)); + } + } + + Ok(None) +} + +fn pg_regress( + pg_config: &PgConfig, + bin: impl AsRef, + dbname: &str, + input_dir: impl AsRef, + tests: &[&DirEntry], +) -> eyre::Result<(ExitStatus, String)> { + if tests.is_empty() { + eyre::bail!("no tests to run"); + } + let test_dir = tests[0].path().parent().unwrap().parent().unwrap().to_path_buf(); + let tests = tests.iter().map(|entry| make_test_name(entry)); + + let mut command = Command::new(bin.as_ref()); + command + .current_dir(test_dir) + .env_remove("PGDATABASE") + .env_remove("PGHOST") + .env_remove("PGPORT") + .env_remove("PGUSER") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .arg("--host") + .arg(pg_config.host()) + .arg("--port") + .arg(pg_config.port()?.to_string()) + .arg("--use-existing") + .arg(&format!("--dbname={dbname}")) + .arg(&format!("--inputdir={}", input_dir.as_ref().display())) + .arg(&format!("--outputdir={}", input_dir.as_ref().display())) + .args(tests); + + #[cfg(not(target_os = "windows"))] + let launcher_script = { + fn make_launcher_script() -> eyre::Result { + use std::os::unix::fs::PermissionsExt; + + // in order to avoid verbose log output being enshrined in expected test output + const LAUNCHER_SCRIPT: &[u8] = b"#! /bin/bash\n$* -v VERBOSITY=terse"; + + let path = PathBuf::from(temp_dir()) + .join(&format!("pgrx-pg_regress-runner-{}.sh", std::process::id())); + let mut tmpfile = File::create(&path)?; + tmpfile.write_all(LAUNCHER_SCRIPT)?; + let mut perms = path.metadata()?.permissions(); + perms.set_mode(0o700); + tmpfile.set_permissions(perms)?; + Ok(path) + } + let launcher_script = make_launcher_script()?; + command.arg(&format!("--launcher={}", launcher_script.display())); + launcher_script + }; + + let command_str = format!("{command:?}"); + println!("{} command {}", " Running".bold().green(), command_str.cyan()); + + let output = command.output()?; + let stdout = decorate_output(&String::from_utf8_lossy(&output.stdout)); + let stderr = decorate_output(&String::from_utf8_lossy(&output.stderr)); + + let cmd_output = if !stdout.is_empty() && !stderr.is_empty() { + format!("{stdout}\n{stderr}") + } else if !stdout.is_empty() { + stdout.to_string() + } else { + stderr.to_string() + }; + + #[cfg(not(target_os = "windows"))] + { + std::fs::remove_file(launcher_script)?; + } + + Ok((output.status, cmd_output)) +} + +fn decorate_output(input: &str) -> String { + let mut decorated = String::with_capacity(input.len()); + for mut line in input.lines() { + let mut is_test_line = false; + if line.starts_with("ok") { + line = line.trim_start_matches("ok"); + decorated.push_str(&format!("{}", "PASS ".bold().bright_green())); + is_test_line = true; + } else if line.starts_with("not ok") { + line = line.trim_start_matches("not ok"); + decorated.push_str(&format!("{}", "FAIL ".bold().bright_red())); + is_test_line = true; + } + + if is_test_line { + fn split_line(line: &str) -> Option<(&str, &str, &str, &str)> { + let mut parts = line.split_whitespace(); + + let num = parts.next()?; + parts.next()?; // throw away the dash (-) + let test_name = parts.next()?; + let execution_time = parts.next()?; + let execution_units = parts.next()?; + Some((num, test_name, execution_time, execution_units)) + } + + if let Some((num, test_name, execution_time, execution_units)) = split_line(line) { + decorated.push_str(&format!("#{num} {test_name} {execution_time}{execution_units}")) + } else { + decorated.push_str(line); + } + } else { + decorated.push_str(&line); + } + decorated.push('\n'); + } + decorated +} + +fn make_test_name(entry: &DirEntry) -> String { + let filename = entry.file_name(); + let filename = filename.to_str().unwrap_or_else(|| panic!("bogus file name: {entry:?}")); + let filename = + filename.split('.').next().unwrap_or_else(|| panic!("invalid filename: `{filename}`")); + filename.to_string() +} + +fn manifest_path_to_sql_tests_path(manifest_path: impl AsRef) -> PathBuf { + let mut path = PathBuf::from(manifest_path.as_ref()); + path.pop(); // pop `Cargo.toml` + path.push("pg_regress"); + path.push("sql"); + path +} + +fn manifest_path_to_expected_tests_output_path(manifest_path: impl AsRef) -> PathBuf { + let mut path = PathBuf::from(manifest_path.as_ref()); + path.pop(); // pop `Cargo.toml` + path.push("pg_regress"); + path.push("expected"); + path +} +fn manifest_path_to_results_output_path(manifest_path: impl AsRef) -> PathBuf { + let mut path = PathBuf::from(manifest_path.as_ref()); + path.pop(); // pop `Cargo.toml` + path.push("pg_regress"); + path.push("results"); + path +} + +fn add_to_git(path: impl AsRef) -> eyre::Result<()> { + if let Ok(git) = which::which("git") { + if is_git_repo(&git) { + if !Command::new(git).arg("add").arg(path.as_ref()).status()?.success() { + panic!("unable to add {} to git", path.as_ref().display()); + } + } + } + Ok(()) +} + +fn is_git_repo(git: impl AsRef) -> bool { + Command::new(git.as_ref()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .arg("rev-parse") + .arg("--is-inside-work-tree") + .status() + .map(|status| status.success()) + .unwrap_or_default() +} diff --git a/cargo-pgrx/src/command/run.rs b/cargo-pgrx/src/command/run.rs index a878cb8cca..079873dd50 100644 --- a/cargo-pgrx/src/command/run.rs +++ b/cargo-pgrx/src/command/run.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; //LICENSE Portions Copyright 2019-2021 ZomboDB, LLC. //LICENSE //LICENSE Portions Copyright 2021-2023 Technology Concepts & Design, Inc. @@ -9,6 +10,7 @@ //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. use crate::command::get::get_property; use crate::command::install::install_extension; +use crate::command::regress::Regress; use crate::command::start::start_postgres; use crate::command::stop::stop_postgres; use crate::manifest::{get_package_manifest, pg_config_and_version}; @@ -17,7 +19,7 @@ use crate::CommandExecute; use eyre::eyre; use owo_colors::OwoColorize; use pgrx_pg_config::{createdb, PgConfig, Pgrx}; -use std::path::Path; +use std::path::{Path, PathBuf}; /// Compile/install extension to a pgrx-managed Postgres instance and start psql #[derive(clap::Args, Debug)] @@ -33,7 +35,7 @@ pub(crate) struct Run { package: Option, /// Path to Cargo.toml #[clap(long)] - manifest_path: Option, + manifest_path: Option, /// Compile for release mode (default is debug) #[clap(long, short)] release: bool, @@ -54,9 +56,30 @@ pub(crate) struct Run { install_only: bool, } -impl CommandExecute for Run { - #[tracing::instrument(level = "error", skip(self))] - fn execute(mut self) -> eyre::Result<()> { +impl From<&Regress> for Run { + fn from(regress: &Regress) -> Self { + Run { + pg_version: regress.pg_version.clone(), + dbname: regress.dbname.clone(), + package: regress.package.clone(), + manifest_path: regress.manifest_path.clone(), + release: regress.release, + profile: regress.profile.clone(), + features: regress.features.clone(), + target: None, + verbose: regress.verbose, + pgcli: false, + install_only: false, + } + } +} + +impl Run { + pub(crate) fn install( + &mut self, + create_database: bool, + postgresql_conf: &HashMap, + ) -> eyre::Result<(PgConfig, String)> { let pgrx = Pgrx::from_config()?; let (package_manifest, package_manifest_path) = get_package_manifest( &self.features, @@ -71,8 +94,8 @@ impl CommandExecute for Run { true, )?; - let dbname = match self.dbname { - Some(dbname) => dbname, + let dbname = match &self.dbname { + Some(dbname) => dbname.clone(), None => get_property(&package_manifest_path, "extname")? .ok_or(eyre!("could not determine extension name"))?, }; @@ -87,12 +110,25 @@ impl CommandExecute for Run { self.package.as_ref(), &package_manifest_path, &dbname, + create_database, &profile, - self.pgcli, &self.features, self.install_only, self.target.as_ref().map(|x| x.as_str()), - ) + postgresql_conf, + )?; + + Ok((pg_config, dbname)) + } +} + +impl CommandExecute for Run { + #[tracing::instrument(level = "error", skip(self))] + fn execute(mut self) -> eyre::Result<()> { + let (pg_config, dbname) = self.install(true, &Default::default())?; + + // run psql + exec_psql(&pg_config, &dbname, self.pgcli) } } @@ -107,11 +143,12 @@ pub(crate) fn run( user_package: Option<&String>, package_manifest_path: &Path, dbname: &str, + create_database: bool, profile: &CargoProfile, - pgcli: bool, features: &clap_cargo::Features, install_only: bool, target: Option<&str>, + postgresql_conf: &HashMap, ) -> eyre::Result<()> { // stop postgres stop_postgres(pg_config)?; @@ -134,15 +171,14 @@ pub(crate) fn run( } // restart postgres - start_postgres(pg_config)?; + start_postgres(pg_config, postgresql_conf)?; // create the named database - if !createdb(pg_config, dbname, false, true, None)? { + if create_database && !createdb(pg_config, dbname, false, true, None)? { println!("{} existing database {}", " Re-using".bold().cyan(), dbname); } - // run psql - exec_psql(pg_config, dbname, pgcli) + Ok(()) } #[cfg(unix)] diff --git a/cargo-pgrx/src/command/start.rs b/cargo-pgrx/src/command/start.rs index eb8215acb9..34dd531de4 100644 --- a/cargo-pgrx/src/command/start.rs +++ b/cargo-pgrx/src/command/start.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; //LICENSE Portions Copyright 2019-2021 ZomboDB, LLC. //LICENSE //LICENSE Portions Copyright 2021-2023 Technology Concepts & Design, Inc. @@ -32,12 +33,19 @@ pub(crate) struct Start { /// Path to Cargo.toml #[clap(long, value_parser)] manifest_path: Option, + + #[clap(long)] + postgresql_conf: Vec, } impl CommandExecute for Start { #[tracing::instrument(level = "error", skip(self))] fn execute(self) -> eyre::Result<()> { - fn perform(me: Start, pgrx: &Pgrx) -> eyre::Result<()> { + fn perform( + me: Start, + pgrx: &Pgrx, + postgresql_conf: &HashMap, + ) -> eyre::Result<()> { let (package_manifest, _) = get_package_manifest( &clap_cargo::Features::default(), me.package.as_ref(), @@ -47,7 +55,7 @@ impl CommandExecute for Start { let (pg_config, _) = pg_config_and_version(pgrx, &package_manifest, me.pg_version, None, false)?; - start_postgres(&pg_config) + start_postgres(&pg_config, postgresql_conf) } let (package_manifest, _) = get_package_manifest( &clap_cargo::Features::default(), @@ -55,22 +63,38 @@ impl CommandExecute for Start { self.manifest_path.clone(), )?; + let postgresql_conf = collect_postgresql_conf_settings(&self.postgresql_conf); let pgrx = Pgrx::from_config()?; if self.pg_version == Some("all".into()) { for v in crate::manifest::all_pg_in_both_tomls(&package_manifest, &pgrx) { let mut versioned_start = self.clone(); versioned_start.pg_version = Some(v?.label()?); - perform(versioned_start, &pgrx)?; + perform(versioned_start, &pgrx, &postgresql_conf)?; } Ok(()) } else { - perform(self, &pgrx) + perform(self, &pgrx, &postgresql_conf) } } } +pub(crate) fn collect_postgresql_conf_settings(settings: &Vec) -> HashMap { + settings + .iter() + .map(|setting| { + let Some((key, value)) = setting.split_once('=') else { + panic!("`--postgresql_conf` setting of `{setting}` is not in the correct format"); + }; + (key.to_string(), value.to_string()) + }) + .collect() +} + #[tracing::instrument(level = "error", skip_all, fields(pg_version = %pg_config.version()?))] -pub(crate) fn start_postgres(pg_config: &PgConfig) -> eyre::Result<()> { +pub(crate) fn start_postgres( + pg_config: &PgConfig, + runtime_conf: &HashMap, +) -> eyre::Result<()> { let datadir = pg_config.data_dir()?; let logfile = pg_config.log_file()?; let bindir = pg_config.bin_dir()?; @@ -92,16 +116,31 @@ pub(crate) fn start_postgres(pg_config: &PgConfig) -> eyre::Result<()> { port.to_string().bold().cyan() ); let pg_ctl = pg_config.pg_ctl_path()?; + + let mut dash_o_args = vec![ + "-i".into(), // listen for tcp connections + "-p".into(), // on this port + port.to_string(), + "-c".into(), // and allow unix socket connections + format!("unix_socket_directories={}", Pgrx::home()?.display()), + ]; + + // user-provided settings to set/override what's in their existing `postgresql.conf` + for (key, value) in runtime_conf { + dash_o_args.push(format!("-c {key}={value}")); + } + let mut command = std::process::Command::new(pg_ctl); command .stdout(Stdio::piped()) .stderr(Stdio::piped()) .arg("start") - .arg(format!("-o -i -p {} -c unix_socket_directories={}", port, Pgrx::home()?.display())) + .arg(format!("-o {}", dash_o_args.join(" "))) .arg("-D") .arg(&datadir) .arg("-l") .arg(&logfile); + #[cfg(target_os = "windows")] { // on windows, created pipes are leaked, so that the command hangs diff --git a/cargo-pgrx/src/manifest.rs b/cargo-pgrx/src/manifest.rs index d941e85714..59e01925a1 100644 --- a/cargo-pgrx/src/manifest.rs +++ b/cargo-pgrx/src/manifest.rs @@ -196,13 +196,13 @@ pub(crate) fn display_version_info(pg_config: &PgConfig, pg_version: &PgVersionS pub(crate) fn get_package_manifest( features: &Features, - package_nane: Option<&String>, + package_name: Option<&String>, manifest_path: Option>, ) -> eyre::Result<(Manifest, PathBuf)> { let metadata = crate::metadata::metadata(features, manifest_path.as_ref()) .wrap_err("couldn't get cargo metadata")?; crate::metadata::validate(manifest_path.as_ref(), &metadata)?; - let package_manifest_path = crate::manifest::manifest_path(&metadata, package_nane) + let package_manifest_path = crate::manifest::manifest_path(&metadata, package_name) .wrap_err("Couldn't get manifest path")?; Ok(( diff --git a/cargo-pgrx/src/templates/gitignore b/cargo-pgrx/src/templates/gitignore index 3906c33241..d3d8f562a1 100644 --- a/cargo-pgrx/src/templates/gitignore +++ b/cargo-pgrx/src/templates/gitignore @@ -4,3 +4,6 @@ *.iml **/*.rs.bk Cargo.lock +pg_regress/results +pg_regress/regression.diffs +pg_regress/regression.out diff --git a/cargo-pgrx/src/templates/setup_out b/cargo-pgrx/src/templates/setup_out new file mode 100644 index 0000000000..4793ad10bc --- /dev/null +++ b/cargo-pgrx/src/templates/setup_out @@ -0,0 +1,3 @@ +-- this setup file is run immediately after the regression database is (re)created +-- the file is optional but you likely want to create the extension +CREATE EXTENSION {name}; diff --git a/cargo-pgrx/src/templates/setup_sql b/cargo-pgrx/src/templates/setup_sql new file mode 100644 index 0000000000..4793ad10bc --- /dev/null +++ b/cargo-pgrx/src/templates/setup_sql @@ -0,0 +1,3 @@ +-- this setup file is run immediately after the regression database is (re)created +-- the file is optional but you likely want to create the extension +CREATE EXTENSION {name}; diff --git a/pgrx-examples/range/.gitignore b/pgrx-examples/range/.gitignore index bdbe7939be..e370d741d6 100644 --- a/pgrx-examples/range/.gitignore +++ b/pgrx-examples/range/.gitignore @@ -5,3 +5,6 @@ **/*.rs.bk Cargo.lock sql/arrays-1.0.sql +pg_regress/results +pg_regress/regression.diffs +pg_regress/regression.out diff --git a/pgrx-examples/range/pg_regress/expected/make_range.out b/pgrx-examples/range/pg_regress/expected/make_range.out new file mode 100644 index 0000000000..73c162d47d --- /dev/null +++ b/pgrx-examples/range/pg_regress/expected/make_range.out @@ -0,0 +1,6 @@ +SELECT range.range(10, 101); + range +---------- + [10,101) +(1 row) + diff --git a/pgrx-examples/range/pg_regress/expected/setup.out b/pgrx-examples/range/pg_regress/expected/setup.out new file mode 100644 index 0000000000..9fbe5026c7 --- /dev/null +++ b/pgrx-examples/range/pg_regress/expected/setup.out @@ -0,0 +1,3 @@ +-- this setup file is run immediately after the regression database is (re)created +-- the file is optional but you likely want to create the extension +CREATE EXTENSION range; diff --git a/pgrx-examples/range/pg_regress/expected/store_ranges.out b/pgrx-examples/range/pg_regress/expected/store_ranges.out new file mode 100644 index 0000000000..f9c03fbdd3 --- /dev/null +++ b/pgrx-examples/range/pg_regress/expected/store_ranges.out @@ -0,0 +1,116 @@ +DROP TABLE IF EXISTS store_ranges; +CREATE TABLE store_ranges +( + id serial8, + r int4range +); +INSERT INTO store_ranges (r) +SELECT range.range(100, 100 + x) +FROM generate_series(0, 100) x; +SELECT * +FROM store_ranges; + id | r +-----+----------- + 1 | empty + 2 | [100,101) + 3 | [100,102) + 4 | [100,103) + 5 | [100,104) + 6 | [100,105) + 7 | [100,106) + 8 | [100,107) + 9 | [100,108) + 10 | [100,109) + 11 | [100,110) + 12 | [100,111) + 13 | [100,112) + 14 | [100,113) + 15 | [100,114) + 16 | [100,115) + 17 | [100,116) + 18 | [100,117) + 19 | [100,118) + 20 | [100,119) + 21 | [100,120) + 22 | [100,121) + 23 | [100,122) + 24 | [100,123) + 25 | [100,124) + 26 | [100,125) + 27 | [100,126) + 28 | [100,127) + 29 | [100,128) + 30 | [100,129) + 31 | [100,130) + 32 | [100,131) + 33 | [100,132) + 34 | [100,133) + 35 | [100,134) + 36 | [100,135) + 37 | [100,136) + 38 | [100,137) + 39 | [100,138) + 40 | [100,139) + 41 | [100,140) + 42 | [100,141) + 43 | [100,142) + 44 | [100,143) + 45 | [100,144) + 46 | [100,145) + 47 | [100,146) + 48 | [100,147) + 49 | [100,148) + 50 | [100,149) + 51 | [100,150) + 52 | [100,151) + 53 | [100,152) + 54 | [100,153) + 55 | [100,154) + 56 | [100,155) + 57 | [100,156) + 58 | [100,157) + 59 | [100,158) + 60 | [100,159) + 61 | [100,160) + 62 | [100,161) + 63 | [100,162) + 64 | [100,163) + 65 | [100,164) + 66 | [100,165) + 67 | [100,166) + 68 | [100,167) + 69 | [100,168) + 70 | [100,169) + 71 | [100,170) + 72 | [100,171) + 73 | [100,172) + 74 | [100,173) + 75 | [100,174) + 76 | [100,175) + 77 | [100,176) + 78 | [100,177) + 79 | [100,178) + 80 | [100,179) + 81 | [100,180) + 82 | [100,181) + 83 | [100,182) + 84 | [100,183) + 85 | [100,184) + 86 | [100,185) + 87 | [100,186) + 88 | [100,187) + 89 | [100,188) + 90 | [100,189) + 91 | [100,190) + 92 | [100,191) + 93 | [100,192) + 94 | [100,193) + 95 | [100,194) + 96 | [100,195) + 97 | [100,196) + 98 | [100,197) + 99 | [100,198) + 100 | [100,199) + 101 | [100,200) +(101 rows) + diff --git a/pgrx-examples/range/pg_regress/sql/make_range.sql b/pgrx-examples/range/pg_regress/sql/make_range.sql new file mode 100644 index 0000000000..98a6ca14f9 --- /dev/null +++ b/pgrx-examples/range/pg_regress/sql/make_range.sql @@ -0,0 +1 @@ +SELECT range.range(10, 101); \ No newline at end of file diff --git a/pgrx-examples/range/pg_regress/sql/setup.sql b/pgrx-examples/range/pg_regress/sql/setup.sql new file mode 100644 index 0000000000..9fbe5026c7 --- /dev/null +++ b/pgrx-examples/range/pg_regress/sql/setup.sql @@ -0,0 +1,3 @@ +-- this setup file is run immediately after the regression database is (re)created +-- the file is optional but you likely want to create the extension +CREATE EXTENSION range; diff --git a/pgrx-examples/range/pg_regress/sql/store_ranges.sql b/pgrx-examples/range/pg_regress/sql/store_ranges.sql new file mode 100644 index 0000000000..f4b692d7b0 --- /dev/null +++ b/pgrx-examples/range/pg_regress/sql/store_ranges.sql @@ -0,0 +1,12 @@ +DROP TABLE IF EXISTS store_ranges; +CREATE TABLE store_ranges +( + id serial8, + r int4range +); + +INSERT INTO store_ranges (r) +SELECT range.range(100, 100 + x) +FROM generate_series(0, 100) x; +SELECT * +FROM store_ranges; \ No newline at end of file diff --git a/pgrx-pg-config/src/lib.rs b/pgrx-pg-config/src/lib.rs index 01b2aea7cb..4192b2cb84 100644 --- a/pgrx-pg-config/src/lib.rs +++ b/pgrx-pg-config/src/lib.rs @@ -369,6 +369,21 @@ impl PgConfig { Ok(path) } + pub fn pg_regress_path(&self) -> eyre::Result { + let mut pgxs_path = self.pgxs_path()?; + pgxs_path.pop(); // pop the `pgxs.mk` file at the end + pgxs_path.pop(); // pop the `makefiles` directory in which it lives + let mut pgregress_path = pgxs_path; + pgregress_path.push("test"); + pgregress_path.push("regress"); + pgregress_path.push("pg_regress"); + Ok(pgregress_path) + } + + pub fn pgxs_path(&self) -> eyre::Result { + self.run("--pgxs").map(PathBuf::from) + } + pub fn data_dir(&self) -> eyre::Result { let mut path = Pgrx::home()?; path.push(format!("data-{}", self.major_version()?)); @@ -710,7 +725,7 @@ pub fn createdb( return Ok(false); } - println!("{} database {}", " Creating".bold().green(), dbname); + println!("{} database {}", " Creating".bold().green(), dbname.bold().cyan()); let createdb_path = pg_config.createdb_path()?; let mut command = if let Some(runas) = runas { let mut cmd = Command::new("sudo"); @@ -760,6 +775,66 @@ pub fn createdb( Ok(true) } +pub fn dropdb( + pg_config: &PgConfig, + dbname: &str, + is_test: bool, + runas: Option, +) -> eyre::Result { + if !does_db_exist(pg_config, dbname)? { + return Ok(false); + } + + println!("{} database {}", " Dropping".bold().green(), dbname.bold().cyan()); + let createdb_path = pg_config.dropdb_path()?; + let mut command = if let Some(runas) = runas { + let mut cmd = Command::new("sudo"); + cmd.arg("-u").arg(runas).arg(createdb_path); + cmd + } else { + Command::new(createdb_path) + }; + command + .env_remove("PGDATABASE") + .env_remove("PGHOST") + .env_remove("PGPORT") + .env_remove("PGUSER") + .arg("-h") + .arg(pg_config.host()) + .arg("-p") + .arg(if is_test { + pg_config.test_port()?.to_string() + } else { + pg_config.port()?.to_string() + }) + .arg(dbname) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + let command_str = format!("{command:?}"); + + let child = command.spawn().wrap_err_with(|| { + format!("Failed to spawn process for dropping database using command: '{command_str}': ") + })?; + + let output = child.wait_with_output().wrap_err_with(|| { + format!( + "failed waiting for spawned process to drop database using command: '{command_str}': " + ) + })?; + + if !output.status.success() { + return Err(eyre!( + "problem running dropdb: {}\n\n{}{}", + command_str, + String::from_utf8(output.stdout).unwrap(), + String::from_utf8(output.stderr).unwrap() + )); + } + + Ok(true) +} + fn does_db_exist(pg_config: &PgConfig, dbname: &str) -> eyre::Result { let mut command = Command::new(pg_config.psql_path()?); command From 54ae1c00cd337bb36fe298fd85ceb6c906c79c93 Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 13:17:40 -0400 Subject: [PATCH 02/10] run the regression tests for our `range` example, as it's the only one that has them --- .github/workflows/tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 8ccdfb6985..8f4a1aae3d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -313,6 +313,9 @@ jobs: - name: Run range example tests run: CARGO_TARGET_DIR="$(pwd)/target" cargo test --manifest-path=pgrx-examples/range/Cargo.toml --features "pg$PG_VER" --no-default-features + - name: Run range example regression tests + run: CARGO_TARGET_DIR="$(pwd)/target" cargo pgrx regress pg$PG_VER --manifest-path=pgrx-examples/range/Cargo.toml + - name: Run schemas example tests run: CARGO_TARGET_DIR="$(pwd)/target" cargo test --manifest-path=pgrx-examples/schemas/Cargo.toml --features "pg$PG_VER" --no-default-features From 023f7c4022c0784ab1a5302f9d58468e02fd6518 Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 13:22:45 -0400 Subject: [PATCH 03/10] oops --- cargo-pgrx/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index 1bfdf25748..260c4d15f4 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -571,11 +571,11 @@ $ tree ├── Cargo.toml ├── README.md ├── pg_regress -│ ├── sql # these are the corresponding test output files +│ ├── sql # these are the individual regression test scripts │ │ ├── make_range.sql │ │ ├── setup.sql │ │ └── store_ranges.sql -│ └── expected # these are the individual regression test scripts +│ └── expected # these are the corresponding test output files │ ├── make_range.out │ ├── setup.out │ └── store_ranges.out From 7bc42cab574ee0f8d97f184d007df22517907bdc Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 13:26:32 -0400 Subject: [PATCH 04/10] no, actually --- cargo-pgrx/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index 260c4d15f4..2b21f1e0f1 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -731,7 +731,8 @@ $ diff -U3 ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/bad.out ~/_wor What you wanted in the .sql file is either ```sql -CREATE TABLE IF NOT EXISTS foo(); +DROP TABLE IF EXISTS foo; +CREATE TABLE foo(); ``` or From 70064aa4121e5a090fa5f6b42573edde5c70530d Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 13:34:46 -0400 Subject: [PATCH 05/10] okay, copilot, you get this one --- cargo-pgrx/src/command/regress.rs | 2 +- cargo-pgrx/src/command/start.rs | 18 ++++++++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/cargo-pgrx/src/command/regress.rs b/cargo-pgrx/src/command/regress.rs index b041e5daf6..5f64d836fa 100644 --- a/cargo-pgrx/src/command/regress.rs +++ b/cargo-pgrx/src/command/regress.rs @@ -245,7 +245,7 @@ impl CommandExecute for Regress { // we purposely want as little noise as possible to end up in the expected test output files self.postgresql_conf.push("client_min_messages=warning".into()); - let postgresql_conf = collect_postgresql_conf_settings(&self.postgresql_conf); + let postgresql_conf = collect_postgresql_conf_settings(&self.postgresql_conf)?; // install the extension let (pg_config, dbname) = Run::from(&self).install(false, &postgresql_conf)?; diff --git a/cargo-pgrx/src/command/start.rs b/cargo-pgrx/src/command/start.rs index 34dd531de4..94904a63c7 100644 --- a/cargo-pgrx/src/command/start.rs +++ b/cargo-pgrx/src/command/start.rs @@ -63,7 +63,7 @@ impl CommandExecute for Start { self.manifest_path.clone(), )?; - let postgresql_conf = collect_postgresql_conf_settings(&self.postgresql_conf); + let postgresql_conf = collect_postgresql_conf_settings(&self.postgresql_conf)?; let pgrx = Pgrx::from_config()?; if self.pg_version == Some("all".into()) { for v in crate::manifest::all_pg_in_both_tomls(&package_manifest, &pgrx) { @@ -78,14 +78,20 @@ impl CommandExecute for Start { } } -pub(crate) fn collect_postgresql_conf_settings(settings: &Vec) -> HashMap { +pub(crate) fn collect_postgresql_conf_settings( + settings: &Vec, +) -> eyre::Result> { settings .iter() .map(|setting| { - let Some((key, value)) = setting.split_once('=') else { - panic!("`--postgresql_conf` setting of `{setting}` is not in the correct format"); - }; - (key.to_string(), value.to_string()) + if let Some((key, value)) = setting.split_once('=') { + Ok((key.to_string(), value.to_string())) + } else { + Err(eyre::eyre!( + "`--postgresql_conf` setting of `{}` is not in the correct format", + setting + )) + } }) .collect() } From ee51c5142c976aa00d80ba4dba7f753705c25e08 Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 16:20:16 -0400 Subject: [PATCH 06/10] introduce a new argument (`--auto`/`-a`) that will both automatically accept the output from new tests and also copy over the output from failed tests to become the new expected output. other little bugfixes an UI cleanups --- cargo-pgrx/README.md | 62 +++++--- cargo-pgrx/src/command/regress.rs | 145 +++++++++++++----- pgrx-examples/range/.gitignore | 6 +- .../pg_regress/expected/make_range.out | 0 .../{ => tests}/pg_regress/expected/setup.out | 0 .../pg_regress/expected/store_ranges.out | 0 .../{ => tests}/pg_regress/sql/make_range.sql | 0 .../{ => tests}/pg_regress/sql/setup.sql | 0 .../pg_regress/sql/store_ranges.sql | 0 9 files changed, 144 insertions(+), 69 deletions(-) rename pgrx-examples/range/{ => tests}/pg_regress/expected/make_range.out (100%) rename pgrx-examples/range/{ => tests}/pg_regress/expected/setup.out (100%) rename pgrx-examples/range/{ => tests}/pg_regress/expected/store_ranges.out (100%) rename pgrx-examples/range/{ => tests}/pg_regress/sql/make_range.sql (100%) rename pgrx-examples/range/{ => tests}/pg_regress/sql/setup.sql (100%) rename pgrx-examples/range/{ => tests}/pg_regress/sql/store_ranges.sql (100%) diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index 2b21f1e0f1..778a2bebb3 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -556,11 +556,12 @@ Options: --no-default-features Do not activate the `default` feature -F, --features Space-separated list of features to activate --postgresql-conf Custom `postgresql.conf` settings in the form of `key=value`, ie `log_min_messages=debug1` + -a, --auto Automatically accept output for new tests *and* overwrite output for existing-but-failing tests -h, --help Print help -V, --version Print version ``` -Regression tests are split into `*.sql` files and `*.out` files. The files themselves are organized into separate directories rooted at `./pg_regress`. +Regression tests are split into `*.sql` files and `*.out` files. The files themselves are organized into separate directories rooted at `./tests/pg_regress`. For example, using our [range example](../pgrx-examples/range/), the directory structure looks like this: @@ -570,15 +571,16 @@ $ tree ├── Cargo.lock ├── Cargo.toml ├── README.md -├── pg_regress -│ ├── sql # these are the individual regression test scripts -│ │ ├── make_range.sql -│ │ ├── setup.sql -│ │ └── store_ranges.sql -│ └── expected # these are the corresponding test output files -│ ├── make_range.out -│ ├── setup.out -│ └── store_ranges.out +├── tests +│ └── pg_regress +│ ├── sql # these are the individual regression test scripts +│ │ ├── make_range.sql +│ │ ├── setup.sql +│ │ └── store_ranges.sql +│ └── expected # these are the corresponding test output files +│ ├── make_range.out +│ ├── setup.out +│ └── store_ranges.out ├── range.control ├── results └── src @@ -606,14 +608,14 @@ $ cargo pgrx regress Copying shared library to ~/.pgrx/13.20/pgrx-install/lib/postgresql/range.so Discovered 9 SQL entities: 0 schemas (0 unique), 9 functions, 0 types, 0 enums, 0 sqls, 0 ords, 0 hashes, 0 aggregates, 0 triggers Rebuilding pgrx_embed, in debug mode, for SQL generation with features pg13 - Compiling range v0.0.0 ( ~/_work/pgrx/pgrx-examples/range) + Compiling range v0.0.0 ( ~/_work/pgrx/tests/pgrx-examples/range) Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.70s Writing SQL entities to ~/.pgrx/13.20/pgrx-install/share/postgresql/extension/range--0.1.0.sql Finished installing range Starting Postgres v13 on port 28813 Re-using existing database range_regress Found 1 new tests, running each individually to create output - Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "example" "--launcher=/tmp/pgrx-pg_regress-runner-2940893.sh" + Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "example" "--launcher=/tmp/pgrx-pg_regress-runner-2940893.sh" ----------- SELECT 1; ?column? @@ -632,8 +634,8 @@ Typing `Y` (or just pressing return) will copy the test output to the proper loc ... test `example` generated the above output: Accept [Y, n]? y - Copying test output to ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/example.out - Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "example" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2940893.sh" + Copying test output to ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/example.out + Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "example" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2940893.sh" (using postmaster on localhost, port 28813) ============== running regression test queries ============== test example ... ok 6 ms @@ -645,6 +647,10 @@ test store_ranges ... ok 20 ms ===================== ``` +Alternatively, you can run `cargo pgrx regress --auto` (or `-a`) to **automatically** accept the output generated by a new test. + +`--auto` will **also** copy the output of **failed **tests to the `./tests/pg_regress/expected/` directory, overwriting the existing expected test output. This is an automated version of blindly accepting different test output as the new, expected output. + ### Things to Know - `setup.sql` is only executed when tests are run for the first time, or the `--resetdb` argument is used @@ -654,8 +660,16 @@ test store_ranges ... ok 20 ms - Postgres' documentation for `pg_regress` [begins here](https://www.postgresql.org/docs/current/regress.html). While pgrx does not support every knob and dial, its organization is largely compatible (PRs welcome to enhance features) - to regenerate the expected test output, delete the `./pg_regress/expected/TEST_NAME.out` file and run `cargo pgrx regress`. You'll be prompted to accept the new output and it'll automatically be run through `git add` - `pg_regress` uses `psql` to run each test and literally diffs the output against the expected output file. pgrx does two things to help eliminate noise in the test output. The first is it sets `client_min_messages=warning` when starting the Postgres instance and it also passes `-v VERBOSITY=terse` through to `psql`. -- when test output includes results from a table (ie, a SELECT statement), watch out for ordering differences between runs. Prefer to always "ORDER BY ..." any such statements. +### Diffing `psql` Output? + +Yes, Postgres' `pg_regress` tool plays each `test_name.sql` file through `psql`, captures the full output, and diffs that output against the test's corresponding `test_name.out` file. It's a good idea for your tests to avoid variability in their output. + +Avoiding variability can mean some simple things like + - ensuring the results of SELECT statements that return multiple rows are always sorted in a predictable/repeatable manner + - avoiding outputting "random" values such as the result of the `random()` function, `txid_current()`, and others + + ### Be Kind to Yourself While always good advice, in the context of individual regression tests, this means that a test should not leave anything behind or should be able to tolerate leftover database objects from previous runs. @@ -679,14 +693,14 @@ $ cargo pgrx regress Copying shared library to ~/.pgrx/13.20/pgrx-install/lib/postgresql/range.so Discovered 9 SQL entities: 0 schemas (0 unique), 9 functions, 0 types, 0 enums, 0 sqls, 0 ords, 0 hashes, 0 aggregates, 0 triggers Rebuilding pgrx_embed, in debug mode, for SQL generation with features pg13 - Compiling range v0.0.0 ( ~/_work/pgrx/pgrx-examples/range) + Compiling range v0.0.0 ( ~/_work/pgrx/tests/pgrx-examples/range) Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.71s Writing SQL entities to ~/.pgrx/13.20/pgrx-install/share/postgresql/extension/range--0.1.0.sql Finished installing range Starting Postgres v13 on port 28813 Re-using existing database range_regress Found 1 new tests, running each individually to create output - Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "bad" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" + Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "bad" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" ----------- CREATE TABLE foo(); @@ -700,8 +714,8 @@ That looks like the perfect output, so we accept it: ... test `bad` generated the above output: Accept [Y, n]? Y - Copying test output to ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/bad.out - Running command cd " ~/_work/pgrx/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/pgrx-examples/range/pg_regress" "bad" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" + Copying test output to ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/bad.out + Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "bad" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" (using postmaster on localhost, port 28813) ============== running regression test queries ============== test bad ... FAILED 5 ms @@ -713,16 +727,16 @@ test store_ranges ... ok 19 ms ====================== The differences that caused some tests to fail can be viewed in the -file " ~/_work/pgrx/pgrx-examples/range/pg_regress/regression.diffs". A copy of the test summary that you see -above is saved in the file " ~/_work/pgrx/pgrx-examples/range/pg_regress/regression.out". +file " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/regression.diffs". A copy of the test summary that you see +above is saved in the file " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/regression.out". ``` And you see the `bad` test immediately failed! To see how it failed, look at the `./pg_regress/regression.diffs` file: ```console -$ diff -U3 ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/bad.out ~/_work/pgrx/pgrx-examples/range/pg_regress/results/bad.out ---- ~/_work/pgrx/pgrx-examples/range/pg_regress/expected/bad.out 2025-05-07 12:15:10.759010127 -0400 -+++ ~/_work/pgrx/pgrx-examples/range/pg_regress/results/bad.out 2025-05-07 12:15:10.775009912 -0400 +$ diff -U3 ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/bad.out ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/results/bad.out +--- ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/bad.out 2025-05-07 12:15:10.759010127 -0400 ++++ ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/results/bad.out 2025-05-07 12:15:10.775009912 -0400 @@ -1 +1,2 @@ CREATE TABLE foo(); +ERROR: relation "foo" already exists diff --git a/cargo-pgrx/src/command/regress.rs b/cargo-pgrx/src/command/regress.rs index 5f64d836fa..e28619147c 100644 --- a/cargo-pgrx/src/command/regress.rs +++ b/cargo-pgrx/src/command/regress.rs @@ -67,6 +67,10 @@ pub(crate) struct Regress { /// Custom `postgresql.conf` settings in the form of `key=value`, ie `log_min_messages=debug1` #[clap(long)] pub(crate) postgresql_conf: Vec, + + /// Automatically accept output for new tests *and* overwrite output for existing-but-failing tests + #[clap(long, short)] + pub(crate) auto: bool, } impl Regress { @@ -99,6 +103,21 @@ impl Regress { Ok((files, setup_file)) } + fn list_results_outputs( + &self, + manifest_path: impl AsRef, + ) -> eyre::Result<(Vec, Option)> { + let results = manifest_path_to_results_output_path(manifest_path); + if !results.exists() { + std::fs::create_dir(&results)?; + } + let mut files = std::fs::read_dir(results)?.collect::, _>>()?; + + let setup_file = Self::organize_files(&mut files, "out"); + + Ok((files, setup_file)) + } + fn organize_files(files: &mut Vec, only: &str) -> Option { // remove any files that don't have `only` as the extension files.retain(|entry| { @@ -138,6 +157,7 @@ impl Regress { &self, manifest_path: impl AsRef, test_result_output: impl AsRef, + auto: bool, ) -> eyre::Result<()> { if !std::io::stdin().is_terminal() { panic!("not a terminal: cannot perform user interaction to accept tests") @@ -151,24 +171,33 @@ impl Regress { .to_string(); let test_output = std::fs::read_to_string(&test_result_output)?; - println!("-----------"); - println!("{}", test_output.white()); - println!("test `{}` generated the above output:", test_name.bold().green()); - eprint!("Accept [Y, n]? "); - - let mut user_input = String::new(); - std::io::stdin().read_line(&mut user_input)?; - let user_input = user_input.trim(); - let variant_suffix: Option; - if user_input == "Y" || user_input == "y" { - variant_suffix = None - } else if user_input.as_bytes()[0] >= b'0' && user_input.as_bytes()[0] <= b'9' { - // currently secret options to create a variant file - // however, postgres requires the original `test_name.out` to also exist - variant_suffix = Some(format!("_{user_input}")); + + if auto { + variant_suffix = None; + println!( + "test `{}` is new, automatically accepting its output as expected", + test_name.bold().green() + ); } else { - std::process::exit(1); + println!("-----------"); + println!("{}", test_output.white()); + println!("test `{}` generated the above output:", test_name.bold().green()); + eprint!("Accept [Y, n]? "); + + let mut user_input = String::new(); + std::io::stdin().read_line(&mut user_input)?; + let user_input = user_input.trim(); + + if user_input == "Y" || user_input == "y" { + variant_suffix = None + } else if user_input.as_bytes()[0] >= b'0' && user_input.as_bytes()[0] <= b'9' { + // currently secret options to create a variant file + // however, postgres requires the original `test_name.out` to also exist + variant_suffix = Some(format!("_{user_input}")); + } else { + std::process::exit(1); + } } let expected_path = manifest_path_to_expected_tests_output_path(manifest_path) @@ -192,22 +221,20 @@ impl Regress { dbname: &str, test_files: &[&DirEntry], output_files: &[&DirEntry], + auto: bool, ) -> eyre::Result<()> { - let test_names = test_files.iter().map(|e| make_test_name(*e)).collect::>(); let output_names = output_files.iter().map(|e| make_test_name(*e)).collect::>(); - if test_names.len() > output_names.len() { - // there are more tests than there are expected outputs, so figure out which ones - // don't have outputs and run them individually to create their outputs - - let new_tests = test_files - .iter() - .filter(|entry| { - let test_name = make_test_name(entry); - !output_names.contains(&test_name) - }) - .collect::>(); + // look for new tests (tests without a corresponding output file) + let new_tests = test_files + .iter() + .filter(|entry| { + let test_name = make_test_name(entry); + !output_names.contains(&test_name) + }) + .collect::>(); + if !new_tests.is_empty() { println!( "{} {} new tests, running each individually to create output", " Found".bold().cyan(), @@ -221,13 +248,40 @@ impl Regress { &dbname, new_test, )? { - self.accept_new_test(&manifest_path, test_result_output)?; + self.accept_new_test(&manifest_path, test_result_output, auto)?; } } } // now that all tests have outputs, run them all - run_tests(pg_config, pgregress_path, dbname, test_files) + let success = run_tests(pg_config, pgregress_path, dbname, test_files)?; + + if !success && auto { + // tests failed, but the user asked to `auto`matically accept their output as new output + let (results_files, _) = self.list_results_outputs(&manifest_path)?; + + println!(); + for entry in results_files { + let filename = + entry.file_name().to_str().expect("filename should be valid UTF8").to_owned(); + let expected_path = + manifest_path_to_expected_tests_output_path(&manifest_path).join(filename); + + let src = std::fs::read_to_string(entry.path())?; + let dst = std::fs::read_to_string(&expected_path)?; + if src != dst { + println!( + "test `{}` failed, automatically promoting its output as", + make_test_name(&entry).bold().bright_red() + ); + std::fs::copy(entry.path(), &expected_path)?; + } + } + + std::process::exit(1); + } + + Ok(()) } } @@ -272,7 +326,11 @@ impl CommandExecute for Regress { // run the setup test, comparing its result to its output (Some(setup_file), Some(_)) => { - run_tests(&pg_config, &pgregress_path, &dbname, &[&setup_file])? + let success = run_tests(&pg_config, &pgregress_path, &dbname, &[&setup_file])?; + + if !success { + panic!("the `{}` test failed", "setup".bold().bright_red()); + } } // create the output for the setup test @@ -285,7 +343,7 @@ impl CommandExecute for Regress { &setup_file, )? { // and ask the user if it's good - self.accept_new_test(&manifest_path, test_result_output)?; + self.accept_new_test(&manifest_path, test_result_output, self.auto)?; } } } @@ -310,6 +368,7 @@ impl CommandExecute for Regress { &dbname, &test_files.iter().collect::>(), &output_files.iter().collect::>(), + self.auto, ) } } @@ -319,9 +378,9 @@ fn run_tests( pg_regress_bin: impl AsRef, dbname: &str, test_files: &[&DirEntry], -) -> eyre::Result<()> { +) -> eyre::Result { if test_files.is_empty() { - return Ok(()); + return Ok(true); } let input_dir = test_files[0].path(); let input_dir = input_dir @@ -334,11 +393,7 @@ fn run_tests( println!("{output}"); - if !status.success() { - std::process::exit(status.code().unwrap_or(1)); - } - - Ok(()) + Ok(status.success()) } fn create_regress_output( @@ -429,8 +484,7 @@ fn pg_regress( launcher_script }; - let command_str = format!("{command:?}"); - println!("{} command {}", " Running".bold().green(), command_str.cyan()); + tracing::trace!("running {command:?}"); let output = command.output()?; let stdout = decorate_output(&String::from_utf8_lossy(&output.stdout)); @@ -442,7 +496,9 @@ fn pg_regress( stdout.to_string() } else { stderr.to_string() - }; + } + .trim() + .to_string(); #[cfg(not(target_os = "windows"))] { @@ -484,6 +540,8 @@ fn decorate_output(input: &str) -> String { decorated.push_str(line); } } else { + let line = line.replace("... FAILED", &"... FAILED".bold().bright_red().to_string()); + let line = line.replace("... ok", &"... ok".bold().bright_green().to_string()); decorated.push_str(&line); } decorated.push('\n'); @@ -502,6 +560,7 @@ fn make_test_name(entry: &DirEntry) -> String { fn manifest_path_to_sql_tests_path(manifest_path: impl AsRef) -> PathBuf { let mut path = PathBuf::from(manifest_path.as_ref()); path.pop(); // pop `Cargo.toml` + path.push("tests"); path.push("pg_regress"); path.push("sql"); path @@ -510,6 +569,7 @@ fn manifest_path_to_sql_tests_path(manifest_path: impl AsRef) -> PathBuf { fn manifest_path_to_expected_tests_output_path(manifest_path: impl AsRef) -> PathBuf { let mut path = PathBuf::from(manifest_path.as_ref()); path.pop(); // pop `Cargo.toml` + path.push("tests"); path.push("pg_regress"); path.push("expected"); path @@ -517,6 +577,7 @@ fn manifest_path_to_expected_tests_output_path(manifest_path: impl AsRef) fn manifest_path_to_results_output_path(manifest_path: impl AsRef) -> PathBuf { let mut path = PathBuf::from(manifest_path.as_ref()); path.pop(); // pop `Cargo.toml` + path.push("tests"); path.push("pg_regress"); path.push("results"); path diff --git a/pgrx-examples/range/.gitignore b/pgrx-examples/range/.gitignore index e370d741d6..7182891c24 100644 --- a/pgrx-examples/range/.gitignore +++ b/pgrx-examples/range/.gitignore @@ -5,6 +5,6 @@ **/*.rs.bk Cargo.lock sql/arrays-1.0.sql -pg_regress/results -pg_regress/regression.diffs -pg_regress/regression.out +tests/pg_regress/results +tests/pg_regress/regression.diffs +tests/pg_regress/regression.out diff --git a/pgrx-examples/range/pg_regress/expected/make_range.out b/pgrx-examples/range/tests/pg_regress/expected/make_range.out similarity index 100% rename from pgrx-examples/range/pg_regress/expected/make_range.out rename to pgrx-examples/range/tests/pg_regress/expected/make_range.out diff --git a/pgrx-examples/range/pg_regress/expected/setup.out b/pgrx-examples/range/tests/pg_regress/expected/setup.out similarity index 100% rename from pgrx-examples/range/pg_regress/expected/setup.out rename to pgrx-examples/range/tests/pg_regress/expected/setup.out diff --git a/pgrx-examples/range/pg_regress/expected/store_ranges.out b/pgrx-examples/range/tests/pg_regress/expected/store_ranges.out similarity index 100% rename from pgrx-examples/range/pg_regress/expected/store_ranges.out rename to pgrx-examples/range/tests/pg_regress/expected/store_ranges.out diff --git a/pgrx-examples/range/pg_regress/sql/make_range.sql b/pgrx-examples/range/tests/pg_regress/sql/make_range.sql similarity index 100% rename from pgrx-examples/range/pg_regress/sql/make_range.sql rename to pgrx-examples/range/tests/pg_regress/sql/make_range.sql diff --git a/pgrx-examples/range/pg_regress/sql/setup.sql b/pgrx-examples/range/tests/pg_regress/sql/setup.sql similarity index 100% rename from pgrx-examples/range/pg_regress/sql/setup.sql rename to pgrx-examples/range/tests/pg_regress/sql/setup.sql diff --git a/pgrx-examples/range/pg_regress/sql/store_ranges.sql b/pgrx-examples/range/tests/pg_regress/sql/store_ranges.sql similarity index 100% rename from pgrx-examples/range/pg_regress/sql/store_ranges.sql rename to pgrx-examples/range/tests/pg_regress/sql/store_ranges.sql From d9298970e58c6abddf1d19268b2a95a288b21a9e Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 16:23:16 -0400 Subject: [PATCH 07/10] cleanup --- cargo-pgrx/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index 778a2bebb3..fc8c86eb84 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -582,7 +582,6 @@ $ tree │ ├── setup.out │ └── store_ranges.out ├── range.control -├── results └── src ├── bin │ └── pgrx_embed.rs From 5a3f7c27e41feffff28a465be15b3c32492f69b5 Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 16:27:19 -0400 Subject: [PATCH 08/10] cleanup --- cargo-pgrx/README.md | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index fc8c86eb84..f525123d33 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -633,7 +633,7 @@ Typing `Y` (or just pressing return) will copy the test output to the proper loc ... test `example` generated the above output: Accept [Y, n]? y - Copying test output to ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/example.out + Copying test output to ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/expected/example.out Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "example" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2940893.sh" (using postmaster on localhost, port 28813) ============== running regression test queries ============== @@ -648,7 +648,7 @@ test store_ranges ... ok 20 ms Alternatively, you can run `cargo pgrx regress --auto` (or `-a`) to **automatically** accept the output generated by a new test. -`--auto` will **also** copy the output of **failed **tests to the `./tests/pg_regress/expected/` directory, overwriting the existing expected test output. This is an automated version of blindly accepting different test output as the new, expected output. +`--auto` will **also** copy the output of **failed ** tests to the `./tests/pg_regress/expected/` directory, overwriting the existing expected test output. This is an automated version of blindly accepting different test output as the new, expected output. ### Things to Know @@ -665,8 +665,9 @@ Alternatively, you can run `cargo pgrx regress --auto` (or `-a`) to **automatica Yes, Postgres' `pg_regress` tool plays each `test_name.sql` file through `psql`, captures the full output, and diffs that output against the test's corresponding `test_name.out` file. It's a good idea for your tests to avoid variability in their output. Avoiding variability can mean some simple things like - - ensuring the results of SELECT statements that return multiple rows are always sorted in a predictable/repeatable manner - - avoiding outputting "random" values such as the result of the `random()` function, `txid_current()`, and others + +- ensuring the results of SELECT statements that return multiple rows are always sorted in a predictable/repeatable manner +- avoiding outputting "random" values such as the result of the `random()` function, `txid_current()`, and others ### Be Kind to Yourself @@ -677,10 +678,10 @@ Each test should be written to "DROP IF EXISTS ... ; CREATE ..." or instead "DRO You can ignore this advice if you **always** run the regression tests with `--resetdb` as you'll have a clean database each time -An example of a poorly designed test would be: +An example of an unkind test might be: ```console -$ echo "CREATE TABLE foo();" > ./pg_regress/sql/bad.sql +$ echo "CREATE TABLE foo();" > ./tests/pg_regress/sql/bad.sql $ cargo pgrx regress Using DefaultFeature("pg13") and `pg_config` from ~/.pgrx/13.20/pgrx-install/bin/pg_config Stopping Postgres v13 @@ -699,7 +700,7 @@ $ cargo pgrx regress Starting Postgres v13 on port 28813 Re-using existing database range_regress Found 1 new tests, running each individually to create output - Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "bad" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" + Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "bad" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" ----------- CREATE TABLE foo(); @@ -713,8 +714,8 @@ That looks like the perfect output, so we accept it: ... test `bad` generated the above output: Accept [Y, n]? Y - Copying test output to ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/bad.out - Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "bad" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" + Copying test output to ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/expected/bad.out + Running command cd " ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress" && env -u PGDATABASE -u PGHOST -u PGPORT -u PGUSER " ~/.pgrx/13.20/pgrx-install/lib/postgresql/pgxs/src/test/regress/pg_regress" "--host" "localhost" "--port" "28813" "--use-existing" "--dbname=range_regress" "--inputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "--outputdir= ~/_work/pgrx/tests/pgrx-examples/range/pg_regress" "bad" "make_range" "store_ranges" "--launcher=/tmp/pgrx-pg_regress-runner-2947999.sh" (using postmaster on localhost, port 28813) ============== running regression test queries ============== test bad ... FAILED 5 ms @@ -726,16 +727,16 @@ test store_ranges ... ok 19 ms ====================== The differences that caused some tests to fail can be viewed in the -file " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/regression.diffs". A copy of the test summary that you see -above is saved in the file " ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/regression.out". +file " ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/regression.diffs". A copy of the test summary that you see +above is saved in the file " ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/regression.out". ``` And you see the `bad` test immediately failed! To see how it failed, look at the `./pg_regress/regression.diffs` file: ```console -$ diff -U3 ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/bad.out ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/results/bad.out ---- ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/expected/bad.out 2025-05-07 12:15:10.759010127 -0400 -+++ ~/_work/pgrx/tests/pgrx-examples/range/pg_regress/results/bad.out 2025-05-07 12:15:10.775009912 -0400 +$ diff -U3 ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/expected/bad.out ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/results/bad.out +--- ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/expected/bad.out 2025-05-07 12:15:10.759010127 -0400 ++++ ~/_work/pgrx/tests/pgrx-examples/range/tests/pg_regress/results/bad.out 2025-05-07 12:15:10.775009912 -0400 @@ -1 +1,2 @@ CREATE TABLE foo(); +ERROR: relation "foo" already exists From 624a83ce70a9ca64c6966d4c745ca1169edc7ea3 Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 16:28:24 -0400 Subject: [PATCH 09/10] cleanup --- cargo-pgrx/README.md | 2 +- cargo-pgrx/src/command/regress.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index f525123d33..a6ba80e3d1 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -556,7 +556,7 @@ Options: --no-default-features Do not activate the `default` feature -F, --features Space-separated list of features to activate --postgresql-conf Custom `postgresql.conf` settings in the form of `key=value`, ie `log_min_messages=debug1` - -a, --auto Automatically accept output for new tests *and* overwrite output for existing-but-failing tests + -a, --auto Automatically accept output for new tests *and* overwrite output for existing-but-failed tests -h, --help Print help -V, --version Print version ``` diff --git a/cargo-pgrx/src/command/regress.rs b/cargo-pgrx/src/command/regress.rs index e28619147c..5da947867b 100644 --- a/cargo-pgrx/src/command/regress.rs +++ b/cargo-pgrx/src/command/regress.rs @@ -68,7 +68,7 @@ pub(crate) struct Regress { #[clap(long)] pub(crate) postgresql_conf: Vec, - /// Automatically accept output for new tests *and* overwrite output for existing-but-failing tests + /// Automatically accept output for new tests *and* overwrite output for existing-but-failed tests #[clap(long, short)] pub(crate) auto: bool, } From bdd0e4b44c2798dbc8ca3a73b2ee9f09e26db8af Mon Sep 17 00:00:00 2001 From: "Eric B. Ridge" Date: Wed, 7 May 2025 16:32:03 -0400 Subject: [PATCH 10/10] cleanup --- cargo-pgrx/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cargo-pgrx/README.md b/cargo-pgrx/README.md index a6ba80e3d1..300ee941eb 100644 --- a/cargo-pgrx/README.md +++ b/cargo-pgrx/README.md @@ -648,7 +648,7 @@ test store_ranges ... ok 20 ms Alternatively, you can run `cargo pgrx regress --auto` (or `-a`) to **automatically** accept the output generated by a new test. -`--auto` will **also** copy the output of **failed ** tests to the `./tests/pg_regress/expected/` directory, overwriting the existing expected test output. This is an automated version of blindly accepting different test output as the new, expected output. +`--auto` will **also** copy the output of **failed** tests to the `./tests/pg_regress/expected/` directory, overwriting the existing expected test output. This is an automated version of blindly accepting different test output as the new, expected output. ### Things to Know