Skip to content

Commit 2a7ad3a

Browse files
authored
Add a psql_verbosity parameter for cargo pgrx regress (pgcentralfoundation#2230)
The pg_regress command uses the default verbosity while cargo pgrx regress uses the terse verbosity. This means that many tests created for pg_regress will fail with cargo regress when HINT or DETAIL messages are printed. This makes harder the process of migrating from pg_regress to cargo pgrx regress. This proposition allows users to define the verbosity. I hesitated between implementing this and a more generic parameter to define a custom .psqlrc file, in the end I choosed the simplest path. --------- Co-authored-by: damien <damien.clochard@dalibo.com>
1 parent 21bcd3f commit 2a7ad3a

File tree

1 file changed

+21
-9
lines changed

1 file changed

+21
-9
lines changed

cargo-pgrx/src/command/regress.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ pub(crate) struct Regress {
6363
pub(crate) features: clap_cargo::Features,
6464
#[clap(from_global, action = clap::ArgAction::Count)]
6565
pub(crate) verbose: u8,
66+
/// verbosity of error reports: default, verbose, terse, or sqlstate
67+
#[clap(long, value_name = "VERBOSITY")]
68+
pub(crate) psql_verbosity: Option<String>,
6669

6770
/// Custom `postgresql.conf` settings in the form of `key=value`, ie `log_min_messages=debug1`
6871
#[clap(long)]
@@ -91,7 +94,7 @@ impl Regress {
9194
let setup_out = expected.join("setup.out");
9295

9396
let Ok(setup_sql) = std::fs::metadata(setup_sql) else { return false; };
94-
let Ok(setup_out) = std::fs::metadata(setup_out) else { return true; }; // there is no output file, so setup.sql is definitely newer
97+
let Ok(setup_out) = std::fs::metadata(setup_out) else { return true; }; // there is no output file, so setup.sql is definitely newer
9598

9699
let Ok(sql_modified) = setup_sql.modified() else { return false; };
97100
let Ok(out_modified) = setup_out.modified() else { return false; };
@@ -278,6 +281,10 @@ impl Regress {
278281
})
279282
.collect::<Vec<_>>();
280283

284+
// The default verbosity is terse in order to avoid verbose log output
285+
// being enshrined in expected test output
286+
let verbosity = &self.psql_verbosity.clone().unwrap_or("terse".into());
287+
281288
if !new_tests.is_empty() {
282289
println!(
283290
"{} {} new tests, running each individually to create output",
@@ -291,14 +298,15 @@ impl Regress {
291298
pgregress_path,
292299
dbname,
293300
new_test,
301+
&verbosity,
294302
)? {
295303
self.accept_new_test(manifest_path, &test_result_output, auto)?;
296304
}
297305
}
298306
}
299307

300308
// now that all tests have outputs, run them all
301-
let success = run_tests(pg_config, pgregress_path, dbname, test_files)?;
309+
let success = run_tests(pg_config, pgregress_path, dbname, test_files, verbosity)?;
302310

303311
if !success && auto {
304312
// tests failed, but the user asked to `auto`matically accept their output as new output
@@ -416,6 +424,7 @@ fn run_tests(
416424
pg_regress_bin: &Path,
417425
dbname: &str,
418426
test_files: &[&DirEntry],
427+
verbosity: &str,
419428
) -> eyre::Result<bool> {
420429
if test_files.is_empty() {
421430
return Ok(true);
@@ -427,7 +436,7 @@ fn run_tests(
427436
.parent()
428437
.expect("test file should be in a directory named `sql/`")
429438
.to_path_buf();
430-
pg_regress(pg_config, pg_regress_bin, dbname, &input_dir, test_files)
439+
pg_regress(pg_config, pg_regress_bin, dbname, &input_dir, test_files, verbosity)
431440
.map(|status| status.success())
432441
}
433442

@@ -437,6 +446,7 @@ fn create_regress_output(
437446
pg_regress_bin: &Path,
438447
dbname: &str,
439448
test_file: &DirEntry,
449+
verbosity: &str,
440450
) -> eyre::Result<Option<PathBuf>> {
441451
let test_name = make_test_name(test_file);
442452
let input_dir = test_file.path();
@@ -446,7 +456,8 @@ fn create_regress_output(
446456
.parent()
447457
.expect("test file should be in a directory named `sql/`")
448458
.to_path_buf();
449-
let status = pg_regress(pg_config, pg_regress_bin, dbname, &input_dir, &[test_file])?;
459+
let status =
460+
pg_regress(pg_config, pg_regress_bin, dbname, &input_dir, &[test_file], verbosity)?;
450461

451462
if !status.success() {
452463
// pg_regress returned with an error code, but that is most likely because the test's output file
@@ -470,6 +481,7 @@ fn pg_regress(
470481
dbname: &str,
471482
input_dir: &Path,
472483
tests: &[&DirEntry],
484+
verbosity: &str,
473485
) -> eyre::Result<ExitStatus> {
474486
if tests.is_empty() {
475487
eyre::bail!("no tests to run");
@@ -498,21 +510,21 @@ fn pg_regress(
498510

499511
#[cfg(not(target_os = "windows"))]
500512
let launcher_script = {
501-
fn make_launcher_script() -> eyre::Result<PathBuf> {
513+
fn make_launcher_script(verbosity: &str) -> eyre::Result<PathBuf> {
502514
use std::os::unix::fs::PermissionsExt;
503515

504-
// in order to avoid verbose log output being enshrined in expected test output
505-
const LAUNCHER_SCRIPT: &[u8] = b"#! /bin/bash\n$* -v VERBOSITY=terse";
516+
let launcher_script =
517+
format!("#! /bin/bash\n$* -v VERBOSITY={}", verbosity.to_string(),).into_bytes();
506518

507519
let path = temp_dir().join(format!("pgrx-pg_regress-runner-{}.sh", std::process::id()));
508520
let mut tmpfile = File::create(&path)?;
509-
tmpfile.write_all(LAUNCHER_SCRIPT)?;
521+
tmpfile.write_all(&launcher_script)?;
510522
let mut perms = path.metadata()?.permissions();
511523
perms.set_mode(0o700);
512524
tmpfile.set_permissions(perms)?;
513525
Ok(path)
514526
}
515-
let launcher_script = make_launcher_script()?;
527+
let launcher_script = make_launcher_script(verbosity)?;
516528
command.arg(format!("--launcher={}", launcher_script.display()));
517529
launcher_script
518530
};

0 commit comments

Comments
 (0)