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 confused print of gp_connections test case. #757

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

avamingli
Copy link
Contributor

I, along with other developers I’ve heard from, occasionally find ourselves puzzled: the test gp_connections clearly succeeded, but the printed results seem somewhat incorrect

When regression succeeds to run gp_connections case, some printed info are confused:

============== running regression test queries  ============== test 
gp_connections ... ok (test process exited with exit code 2)

While the test passed, but the message seems to tell us that the process exit unexpectedly.

I dig into this and found: the case is designed to exit with code 2 which is expected: failed to connect a primary db.

However the message may make developers confused and try to find something wrong but actually not.
And after a failed connect, the script will exit, there is no chance to do something to make amends.

Hacked in pg_regress to ignore gp_connections exit code info.

============== running regression test queries  ==============
test gp_connections               ... ok         2500 ms

This fix doesn't have an impact on test case behavior: if there is diffs between sql and expected files, the case failed as expected.

Authored-by: Zhang Mingli [email protected]

Fixes #ISSUE_Number

What does this PR do?

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


When regression succeeds to run gp_connections case, some
printed info are confused:

============== running regression test queries  ==============
test gp_connections ... ok (test process exited with exit code 2)

While the test passed, but the message seems to tell us that the
process exit unexpectedly.
The case is designed to exit with code 2 which is expected: failed
to connect a primary db.
However the message may make developers confused and try to find
something wrong but actually not.
And after a failed connect, the script will exit, there is no chance
to do something to make amends.

Hacked in pg_regress to ignore gp_connections exit code info.

============== running regression test queries  ==============
test gp_connections               ... ok         2500 ms

This fix doesn't have an impact on test case behavior: if there is
diffs between sql and expected files, the case failed as expected.

Authored-by: Zhang Mingli [email protected]
@avamingli avamingli requested a review from gfphoenix78 December 5, 2024 08:40
@avamingli avamingli requested a review from yjhjstz December 13, 2024 09:08
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.

1 participant