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

Support protocol packages larger than 16 MiB #47 #197

Open
wants to merge 2 commits into
base: 0.7.x
Choose a base branch
from

Conversation

dmarkic
Copy link

@dmarkic dmarkic commented Apr 18, 2024

Reopened PR (#166)

There are currently no tests performed for this.

@SimonFrings

Should the functionality be tested in tests/MysqlClientTest.php or only in tests/Io/ParserTest.php or in both?

We had some issues before with testing on real Mysql server as max_allowed_packet has to be set above 16M limit in order to test packet splitting. When default is used (16M), you cannot really send a packet of sufficient size to start packet splitting.

@SimonFrings
Copy link
Contributor

@dmarkic Thanks for reopening and taking another shot at this 👍

Should the functionality be tested in tests/MysqlClientTest.php or only in tests/Io/ParserTest.php or in both?

Well, we have to test this on the unit side (tests/Io/ParserTest.php) to confirm the Parser handling package splitting as expected and on the functional side as well. I'm not sure if the tests/MysqlClientTest.php is the right place for functional tests, I think we have to focus on the tests/ResultQueryTest.php like in #166, or am I wrong here 😅

We had some issues before with testing on real Mysql server as max_allowed_packet has to be set above 16M limit in order to test packet splitting. When default is used (16M), you cannot really send a packet of sufficient size to start packet splitting.

I think we have to increase max_allowed_packet as we did in https://github.com/friends-of-reactphp/mysql/pull/166/files#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03f

@dmarkic
Copy link
Author

dmarkic commented Apr 21, 2024

This is now a complete test. src/Io/Parser.php coverage is 100% (excluding debug() method).
It tests both sending and receiving of packets same or larger than 16MiB.

@dmarkic
Copy link
Author

dmarkic commented Sep 25, 2024

Hello,

are there any problems with this or why is there no progres?

Kind regards,
Dejan Markic

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.

2 participants