8000 Adding helper functions that reduce code duplications in TAP tests by shahidullah79 · Pull Request #263 · percona/postgres · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Adding helper functions that reduce code duplications in TAP tests #263

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: TDE_REL_17_STABLE
Choose a base branch
from
Open

Adding helper functions that reduce code duplications in TAP tests #263

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14f8ff4
Adding some helper functions that reduce code duplications in multipl…
shahidullah79 Apr 25, 2025
9b14940
Fixing indentation issue
shahidullah79 Apr 25, 2025
a693649
Fixing perltidy issue
shahidullah79 Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions contrib/pg_tde/t/pgtde.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,65 @@ sub compare_results
return compare($expected_filename_with_path, $out_filename_with_path);
}

# Common TDE helpers

# Check if the encryption status of a table is as expected and return 't' or 'f'
sub check_encryption_status
{
my ($node, $table_name, $expected) = @_;
my $result =
safe_psql('postgres', "SELECT pg_tde_is_encrypted('$table_name')");
append_to_result_file($node->name . ": encryption check result for $table_name = $result");
is($result, $expected, "Check encryption status for '$table_name' on " . $node->name);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest not having such a test assertion (anything we use from Test::More) as part of a helper function; this should be handled in the main test file. That will make it easier to understand and follow the test case.

}

# Set up pg_tde extension and add a global key provider and set the server key
sub setup_pg_tde_global_environment
{
my ($node, $key_name, $provider_name, $provider_path) = @_;
psql($node, 'postgres', 'CREATE EXTENSION IF NOT EXISTS pg_tde;');
psql($node, 'postgres',
"SELECT pg_tde_add_global_key_provider_file('$provider_name', '$provider_path');");
psql($node, 'postgres',
"SELECT pg_tde_set_server_key_using_global_key_provider('$key_name', '$provider_name');");
}

# Set up pg_tde extension and add a database key provider and set the database key
sub setup_pg_tde_db_environment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and the previous function: maybe _key or _keyring instead of _environment? E.g. setup_pg_tde_global_key and setup_pg_tde_db_key

{
my ($node, $key_name, $provider_name, $provider_path) = @_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases in tests nobody cares about the key and provider name, and path. So maybe have defaults like:

my $testname = basename($0);
$testname =~ s/\.[^.]+$//;

my $key_name = $key_name . '_db_key';  
my $provider_name = $testname . '_provider';
my $provider_path = '/tmp/' . $testname . '.per';

And then in most cases you call much more convenient setup_pg_tde_db_key($node)

psql($node, 'postgres', 'CREATE EXTENSION IF NOT EXISTS pg_tde;');
psql($node, 'postgres',
"SELECT pg_tde_add_database_key_provider_file('$provider_name', '$provider_path');");
psql($node, 'postgres',
"SELECT pg_tde_set_key_using_database_key_provider('$key_name', '$provider_name');");
}

# Set up pg_tde in postgresql.conf
sub enable_pg_tde_in_conf
{
my ($node) = @_;
$node->append_conf('postgresql.conf', "shared_preload_libraries = 'pg_tde'");
}

# Set default table access method to tde_heap
sub set_default_table_am_tde_heap
{
my ($node) = @_;
$node->append_conf('postgresql.conf', "default_table_access_method = 'tde_heap'");
}
Comment on lines +119 to +130
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about those two functions. I don't think anyone would memorize this func names, hence they would be copied from the existing tests anyway. But then it's not a bit difference which line is to copy (I mean $node->append_conf('postgresql.conf', "default_table_access_method = 'tde_heap'"); or set_default_table_am_tde_heap($node)). But append_conf( is more clear of what it does. However, I don't have super strong opinion about that.


# Set pg_tde.wal_encrypt and restart the server
sub set_wal_encryption_and_restart
{
my ($node, $value) = @_;

die "Invalid value for wal_encrypt: must be 'on' or 'off'\n"
unless $value eq 'on' || $value eq 'off';

psql($node, 'postgres', "ALTER SYSTEM SET pg_tde.wal_encrypt = $value;");
append_to_result_file("-- server restart with wal encryption = $value");
$node->restart;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having such helper functions for scaffolding purposes is a good idea, will reduce the clutter in the test files and look better.

1;
Loading
0