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

Fix error in run_pgaudit_test() #596

Merged
merged 1 commit into from
Oct 9, 2024
Merged

Conversation

SlouchyButton
Copy link
Contributor

@SlouchyButton SlouchyButton commented Oct 8, 2024

Initially command test was missing, but after it was added test failed and crashed in a way that no more tests were run. This behavior is known issue tracked here in issue #537

Problem is that DOCKER_EXTRA_ARGS variable is set already which it shouldn't be, so the test command returns 0, so no cp happens and we get the crash.

Variable is empty, but apparently do exist as
test -v DOCKER_EXTRA_ARGS is TRUE.

Introduced by PR #399

Fixes: #595

Initially command `test was missing`, but after it was added
test failed and crashed in a way that no more tests were run.
This behavior is known issue tracked here in issue sclorg#537

Problem is that `DOCKER_EXTRA_ARGS` variable is set already
which it shouldn't be, so the `test` command returns 0,
so no `cp` happens and we get the crash.

Variable is empty, but apparently do exist as
`test -v DOCKER_EXTRA_ARGS` is `TRUE`.

Introduced by PR sclorg#399
@SlouchyButton SlouchyButton requested a review from phracek October 8, 2024 14:15
@SlouchyButton
Copy link
Contributor Author

[test]

@@ -934,7 +934,7 @@ run_pgaudit_test()
# create a dir for config
config_dir=$(mktemp -d --tmpdir pg-hook-volume.XXXXX)
add_cleanup_command /bin/rm -rf "$config_dir"
-v DOCKER_EXTRA_ARGS || cp -r "$test_dir"/examples/pgaudit/* "$config_dir"
test -n "$DOCKER_EXTRA_ARGS" || cp -r "$test_dir"/examples/pgaudit/* "$config_dir"
Copy link
Member

Choose a reason for hiding this comment

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

This means if the string is not empty. That's correct. Definition is here https://github.com/sclorg/postgresql-container/blob/master/test/run_test#L41. It means, that if variable is defined, then pgaudit is copied. Am I right?

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This means if the string is not empty.

This is correct

if variable is defined, then pgaudit is copied

Yes as long as the variable is empty, if it's not, it's not copied.

Copy link
Member

@phracek phracek left a comment

Choose a reason for hiding this comment

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

Only if my expectations or understanding of the code is correct.

@phracek
Copy link
Member

phracek commented Oct 8, 2024

[test]

Copy link

github-actions bot commented Oct 8, 2024

Testing Farm results

namecomposearchstatusstarted (UTC)timelogs
CentOS Stream 10 - 16CentOS-Stream-10x86_64✅ passed08.10.2024 14:53:277min 41stest pipeline
Fedora - 16Fedora-latestx86_64✅ passed08.10.2024 14:53:329min 58stest pipeline
CentOS Stream 9 - 13CentOS-Stream-9x86_64✅ passed08.10.2024 14:53:1510min 11stest pipeline
CentOS Stream 9 - 15CentOS-Stream-9x86_64✅ passed08.10.2024 14:53:3011min 54stest pipeline
Fedora - 15Fedora-latestx86_64✅ passed08.10.2024 14:53:2611min 35stest pipeline
CentOS Stream 9 - 16CentOS-Stream-9x86_64✅ passed08.10.2024 14:53:3011min 51stest pipeline
RHEL9 - 13RHEL-9.4.0-Nightlyx86_64✅ passed08.10.2024 14:53:1614min 27stest pipeline
RHEL9 - 15RHEL-9.4.0-Nightlyx86_64✅ passed08.10.2024 14:53:2617min 2stest pipeline
RHEL9 - 16RHEL-9.4.0-Nightlyx86_64✅ passed08.10.2024 14:53:2817min 41stest pipeline
RHEL8 - 12RHEL-8.10.0-Nightlyx86_64✅ passed08.10.2024 14:53:1718min 44stest pipeline
RHEL8 - 13RHEL-8.10.0-Nightlyx86_64✅ passed08.10.2024 14:53:1720min 9stest pipeline
RHEL8 - 15RHEL-8.10.0-Nightlyx86_64✅ passed08.10.2024 14:53:2720min 21stest pipeline
RHEL8 - 16RHEL-8.10.0-Nightlyx86_64✅ passed08.10.2024 14:53:2722min 41stest pipeline

@phracek phracek merged commit 355d42f into sclorg:master Oct 9, 2024
14 checks passed
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

Successfully merging this pull request may close these issues.

run_pgaudit_test crashing the test suite
2 participants