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 tests 4.1.3, 4.1.4, 4.2.1, 4.2.2, 4.2.3, 4.2.4 and way to process data in woketo #28

Merged
merged 2 commits into from
Nov 7, 2016

Conversation

Nek-
Copy link
Member

@Nek- Nek- commented Nov 6, 2016

Please notice that the new way of processing data is not great in term of maintainability. This is an echo to #23 .

This PR should close #27

image
image

Please notice it also fixes some tests in 9.x.

@Nek- Nek- added this to the Version 1 milestone Nov 6, 2016
// return '';
// }
//
// return $data;
Copy link
Contributor

Choose a reason for hiding this comment

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

no code in comments please


public function __construct($maxLength, $message = 'The frame is too big to be processed.')
Copy link
Contributor

Choose a reason for hiding this comment

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

no type ?

@@ -8,6 +8,7 @@
* on the root directory of this project
*/

declare(strict_types=1);
Copy link
Contributor

Choose a reason for hiding this comment

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

why ? no good for performance ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Because sometimes it's really important. (and it is for some parts of woketo)

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand but it is in a central class, so it should be interesting a load tests for the impact. But ok

This commit modify the way of processing binary entry frames.

Old way: taking binary frame as websocket frame (and bufferize if it was
not complete).

New way: it take a binary frame as one or many frames composing one or
many messages and bufferize if frames are not complete.

This new behavior makes autobahn execute new tests that was crashing
woketo. The fix is taking 4 bytes for the mask (sometimes it was less
because of a wrong usage of the BitManipulation class).

Switch to usage of generators
@Nek- Nek- force-pushed the fix/wstest-4.1.3 branch from 3d0c334 to 8dc874d Compare November 6, 2016 23:59
@Nek- Nek- changed the title Fix test 4.1.3 and way to process data in woketo Fix test 4.1.3, 4.1.4, 4.2.1, 4.2.2, 4.2.3, 4.2.4 and way to process data in woketo Nov 7, 2016
@Nek- Nek- changed the title Fix test 4.1.3, 4.1.4, 4.2.1, 4.2.2, 4.2.3, 4.2.4 and way to process data in woketo Fix tests 4.1.3, 4.1.4, 4.2.1, 4.2.2, 4.2.3, 4.2.4 and way to process data in woketo Nov 7, 2016
@folliked folliked merged commit e22a05d into master Nov 7, 2016
@folliked folliked deleted the fix/wstest-4.1.3 branch November 7, 2016 00:15
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.

Solve wstest 4.1.3
2 participants