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

Improve multipart limits test #491

Merged

Conversation

WyriHaximus
Copy link
Member

The PR introducing this test assumed time would be enough to accurately predict behavior. This commit changes it to using introspection to check the exact state of the parser and expected state once finished parsing a multipart request body. To accomplish that it was required to make the cursor in the file an object property, so it can be inspected using reflection.

@WyriHaximus WyriHaximus added this to the v1.9.0 milestone Mar 2, 2023
The PR introducing this test assumed time would be enough to accurately predict behavior. This commit changes it to using introspection to check the exact state of the parser and expected state once finished parsing a multipart request body. To accomplish that it was required to make the cursor in the file an object property, so it can be inspected using reflection.
@WyriHaximus WyriHaximus force-pushed the 1.x-improve-multipart-limits-test branch from b33cd9f to 2942434 Compare March 2, 2023 15:55
Copy link
Member

@SimonFrings SimonFrings left a comment

Choose a reason for hiding this comment

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

I like that we're no longer dependent on the time measurement here 👍

Copy link
Member

@clue clue left a comment

Choose a reason for hiding this comment

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

Not exactly the most beautiful changeset, but it's an improvement over the previous version, so… :shipit: Thanks @WyriHaximus!

@clue clue merged commit 5d2df79 into reactphp:1.x Apr 14, 2023
@WyriHaximus
Copy link
Member Author

Done worse and done better, not proud of it but gets the job done 🖖 .

@WyriHaximus WyriHaximus deleted the 1.x-improve-multipart-limits-test branch April 16, 2023 19:49
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