Skip to content

Fix: pdo pgsql test 12423 #12454

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 occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Oct 23, 2023
Merged

Fix: pdo pgsql test 12423 #12454

merged 1 commit into from
Oct 23, 2023

Conversation

SakiTakamachi
Copy link
Member

@SakiTakamachi SakiTakamachi commented Oct 17, 2023 •
edited
Loading

@SakiTakamachi SakiTakamachi force-pushed the fix/pdo_pgsql_test_12423_2 branch from 90e98a9 to 95cf409 Compare October 17, 2023 09:16
@devnexen
Copy link
Member

Whenever you can @mbeccati let us know if this work for you, cheers.

@mbeccati
Copy link
Contributor

I had tested a previous version of the PR and it was ok, thanks!

devnexen
devnexen reviewed Oct 22, 2023
View reviewed changes
ext/pdo_pgsql/tests/gh12423.phpt Outdated

echo "dsn without credentials / correct user / correct password\n";
try {
$db = new PDO($dsn, $user, $password, [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION]);
echo "Connected.\n\n";
} catch (PDOException $e) {
echo $e->getMessage();
echo $e->getMessage()."\n";
Copy link
Member

Choose a reason for hiding this comment

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

nit: While at it, would be best to use PHP_EOL instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you, I fixed it

@SakiTakamachi
Optimized pdo_pgsql connection test
eb0fe60 
Fix regular expression

fix to use PHP_EOL
@SakiTakamachi SakiTakamachi force-pushed the fix/pdo_pgsql_test_12423_2 branch from 95cf409 to eb0fe60 Compare October 23, 2023 01:06
@SakiTakamachi
Copy link
Member Author

My test is failing, but it might be flaky.

@mbeccati
Copy link
Contributor

The build error doesn't seem related to this PR though:

ERROR: The system cannot find the file specified.
Scanning the drive for archives:
Firebird.zip

@devnexen
Copy link
Member

No it is not related at all indeed. Will review a bit later today. Thanks.

@SakiTakamachi
Copy link
Member Author

SakiTakamachi commented Oct 23, 2023 •
edited
Loading

I was about to write that the flaky test may have failed in an unrelated place. I couldn't explain it well in English, thank you for check.

@devnexen devnexen merged commit cf701fb into php:master Oct 23, 2023
@SakiTakamachi
Copy link
Member Author

Thank you!

@mbeccati
Copy link
Contributor

For reference, CI green again: https://revive.beccati.com/bamboo/browse/PHP-SRC-3566/

@SakiTakamachi
Copy link
Member Author

Thank you, I feel relieved.

@SakiTakamachi SakiTakamachi deleted the fix/pdo_pgsql_test_12423_2 branch November 21, 2023 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@devnexen devnexen devnexen approved these changes

Assignees
No one assigned
Labels
Extension: pdo_pgsql
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SakiTakamachi @devnexen @mbeccati