Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to test GucContext::SuBackend #1906

Open
mrl5 opened this issue Oct 9, 2024 · 7 comments
Open

how to test GucContext::SuBackend #1906

mrl5 opened this issue Oct 9, 2024 · 7 comments

Comments

@mrl5
Copy link
Contributor

mrl5 commented Oct 9, 2024

hello, let's assume that I have this GUC initialization logic:

use pgrx::*;
use std::ffi::CStr;

pub static MY_FOO_RUNTIME_PARAM: &str = "my.foo";
pub static MY_FOO: GucSetting<Option<&'static CStr>> = GucSetting::<Option<&'static CStr>>::new(None);

pub fn init() {
    GucRegistry::define_string_guc(
        MY_FOO_RUNTIME_PARAM,
        "My Foo",
        "That can be FooBar",
        &MY_FOO,
        GucContext::SuBackend,
        GucFlags::NOT_WHILE_SEC_REST
    );
}
  1. How can I properly set it for unit tests? I've tried with some util function that works for GucContext::Suset:
fn set_my_foo_in_guc(value: String) {
    Spi::run(format!("SET {} = '{}'", MY_FOO_RUNTIME_PARAM, value).as_str()).unwrap();
}

but - as expected - I'm getting

Client Error:
parameter "my.foo" cannot be set after connection start
postgres location: guc.c
  1. For manual tests, how can set this GUC param in cargo pgrx run pg16?
@workingjubilee
Copy link
Member

workingjubilee commented Oct 9, 2024

Uhh, I believe that this is impossible to unit test with pg_test as it currently is because pg_test runs tests in transactions, which means it can only test things that work if you run the test against the same backend in two different transactions. It has been a while since I looked at the test code so I may be misremembering slightly.

@workingjubilee
Copy link
Member

workingjubilee commented Oct 9, 2024

It seems plausible that pgrx-tests could be taught how to run tests in connections and thus, if I understand correctly, test this by opening multiple connections, right? But this would require Effort, as it would then have to learn not only the connection management... it might even know that already... but it would also need to be taught how to set the configuration on initial connection for the test.

@workingjubilee
Copy link
Member

workingjubilee commented Oct 9, 2024

As for manual testing, I suppose you can simply set it in the configuration file? That's what I usually have done.

@mrl5
Copy link
Contributor Author

mrl5 commented Oct 9, 2024

with manual test I was able to make it running with:

export PGOPTIONS="-c my.foo=bar"
cargo pgrx run pg16
SHOW my.foo;
 my.foo 
--------
 bar
(1 row)

with unit tests

export PGOPTIONS="-c my.foo=bar"
cargo test

it seems to be missing but I'm still not 100% sure

@workingjubilee
Copy link
Member

I don't believe we pass PGOPTIONS on to the postgres process.

@workingjubilee
Copy link
Member

Actually it seems it's not on the list of env vars we strip out. Weird. Should work, then.

@workingjubilee
Copy link
Member

this is where we start up postgres:

} else {
Command::new(postmaster_path)
};
command
.arg("-D")
.arg(get_pgdata_path()?.to_str().unwrap())
.arg("-h")
.arg(pg_config.host())
.arg("-p")
.arg(pg_config.test_port().expect("unable to determine test port").to_string())
// Redirecting logs to files can hang the test framework, override it
.args(["-c", "log_destination=stderr", "-c", "logging_collector=off"])
.stdout(Stdio::inherit())
.stderr(Stdio::piped());
let command_str = format!("{command:?}");
// start Postgres and monitor its stderr in the background
// also notify the main thread when it's ready to accept connections
let session_id = monitor_pg(command, command_str, loglines);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants