I wrote: > I happened to notice this stuff getting added to my .psql_history: > \echo background_psql: ready > SET password_encryption='scram-sha-256'; > ; > \echo background_psql: QUERY_SEPARATOR > SET scram_iterations=42; > ; > \echo background_psql: QUERY_SEPARATOR > \password scram_role_iter > \q
> After grepping for these strings, this is evidently the fault of > src/test/authentication/t/001_password.pl by way of BackgroundPsql.pm, > which fires up an interactive psql run that is not given the -n switch. > Currently the only other user of interactive_psql() seems to be > psql/t/010_tab_completion.pl, which avoids this problem by > explicitly redirecting the history file. We could have 001_password.pl > do likewise, or we could have it pass the -n switch, but I think we're > going to have this problem resurface repeatedly if we leave it to the > outer test script to remember to do it. After studying this some more, my conclusion is that BackgroundPsql.pm failed to borrow as much as it should have from 010_tab_completion.pl. Specifically, we want all the environment-variable changes that that script performed to be applied in any test using an interactive psql. Maybe ~/.inputrc and so forth would never affect any other test scripts, but that doesn't seem like a great bet. So that leads me to the attached proposed patch. regards, tom lane
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl index 4cd0fa4680..f2d2809aef 100644 --- a/src/bin/psql/t/010_tab_completion.pl +++ b/src/bin/psql/t/010_tab_completion.pl @@ -46,25 +46,6 @@ $node->safe_psql('postgres', . "CREATE TYPE enum1 AS ENUM ('foo', 'bar', 'baz', 'BLACK');\n" . "CREATE PUBLICATION some_publication;\n"); -# Developers would not appreciate this test adding a bunch of junk to -# their ~/.psql_history, so be sure to redirect history into a temp file. -# We might as well put it in the test log directory, so that buildfarm runs -# capture the result for possible debugging purposes. -my $historyfile = "${PostgreSQL::Test::Utils::log_path}/010_psql_history.txt"; -$ENV{PSQL_HISTORY} = $historyfile; - -# Another pitfall for developers is that they might have a ~/.inputrc -# file that changes readline's behavior enough to affect this test. -# So ignore any such file. -$ENV{INPUTRC} = '/dev/null'; - -# Unset $TERM so that readline/libedit won't use any terminal-dependent -# escape sequences; that leads to way too many cross-version variations -# in the output. -delete $ENV{TERM}; -# Some versions of readline inspect LS_COLORS, so for luck unset that too. -delete $ENV{LS_COLORS}; - # In a VPATH build, we'll be started in the source directory, but we want # to run in the build directory so that we can use relative paths to # access the tab_comp_dir subdirectory; otherwise the output from filename @@ -91,8 +72,13 @@ open $FH, ">", "tab_comp_dir/afile456" print $FH "other stuff\n"; close $FH; +# Arrange to capture, not discard, the interactive session's history output. +# Put it in the test log directory, so that buildfarm runs capture the result +# for possible debugging purposes. +my $historyfile = "${PostgreSQL::Test::Utils::log_path}/010_psql_history.txt"; + # fire up an interactive psql session -my $h = $node->interactive_psql('postgres'); +my $h = $node->interactive_psql('postgres', history_file => $historyfile); # Simple test case: type something and see if psql responds as expected sub check_completion diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index f43e54282f..4d207730c9 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -2128,14 +2128,17 @@ Errors occurring later are the caller's problem. Be sure to "quit" the returned object when done with it. -The only extra parameter currently accepted is - =over =item extra_params => ['--single-transaction'] If given, it must be an array reference containing additional parameters to B<psql>. +=item history_file => B<path> + +Cause the interactive B<psql> session to write its command history to B<path>. +If not given, the history is sent to /dev/null. + =back This requires IO::Pty in addition to IPC::Run. @@ -2148,6 +2151,29 @@ sub interactive_psql local %ENV = $self->_get_env(); + # Since the invoked psql will believe it's interactive, it will use + # readline/libedit if available. We need to adjust some environment + # settings to prevent unwanted side-effects. + + # Developers would not appreciate tests adding a bunch of junk to + # their ~/.psql_history, so redirect readline history somewhere else. + # If the calling script doesn't specify anything, just bit-bucket it. + my $history_file = $params{history_file}; + $history_file ||= '/dev/null'; + $ENV{PSQL_HISTORY} = $history_file; + + # Another pitfall for developers is that they might have a ~/.inputrc + # file that changes readline's behavior enough to affect the test. + # So ignore any such file. + $ENV{INPUTRC} = '/dev/null'; + + # Unset $TERM so that readline/libedit won't use any terminal-dependent + # escape sequences; that leads to way too many cross-version variations + # in the output. + delete $ENV{TERM}; + # Some versions of readline inspect LS_COLORS, so for luck unset that too. + delete $ENV{LS_COLORS}; + my @psql_params = ( $self->installed_command('psql'), '-XAt', '-d', $self->connstr($dbname));