Skip to content

Avoid multiple connects in SKIPIF sections #15470

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

Merged
merged 3 commits into from
Aug 18, 2024

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 17, 2024

Besides checking for the ability to connect to the MySQL server, some tests require additional checks (e.g. to be able to check for the server's version) as skip condition. There is no need, though, to connect twice; instead we introduce mysqli_connect_or_skip() in test_helpers.inc, which die()s with an appropriate error message, if the connection can't be established, or returns the connection link otherwise.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

The idea looks sensible to me

cmb69 and others added 3 commits August 18, 2024 11:36
Besides checking for the ability to connect to the MySQL server, some
tests require additional checks (e.g. to be able to check for the
server's version) as skip condition.  There is no need, though, to
connect twice; instead we introduce `mysqli_connect_or_skip()` in
test_helpers.inc, which `die()`s with an appropriate error message, if
the connection can't be established, or returns the connection link
otherwise.
Co-authored-by: Kamil Tekiela <tekiela246@gmail.com>
@cmb69 cmb69 force-pushed the cmb/mysqli_connect_or_skip branch from bb5242c to c8003b4 Compare August 18, 2024 09:47
@cmb69 cmb69 requested a review from kamil-tekiela August 18, 2024 09:58
@cmb69 cmb69 merged commit 7a9120e into php:master Aug 18, 2024
9 checks passed
@cmb69 cmb69 deleted the cmb/mysqli_connect_or_skip branch August 18, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants