[HttpFoundation] Add tests for uncovered sections #57714
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
alexandre-daubois
commented
Jul 12, 2024
•
| Q | A |
|---|---|
| Branch? | 5.4 |
| Bug fix? | no |
| New feature? | no |
| Deprecations? | no |
| Issues | - |
| License | MIT |
|
Great, I propose to merge such PRs in lower branches. 😎 |
|
That's what I went for at first but I remembered that this topic was raised in my other PRs about test coverage, where it's merged in the highest branch possible 😄 For example: #52771 (comment), #52746 (review) |
|
Yes, same for me 😄 |
OskarStark
reviewed
Jul 15, 2024
src/Symfony/Component/HttpFoundation/Tests/RequestMatcher/QueryParameterRequestMatcherTest.php
Outdated
Show resolved
Hide resolved
c7894e5 to
3a7ae65
Compare
|
There's nothing wrong with increasing test coverage on LTS branches. I'd say, go for it. |
3a7ae65 to
a2dc67b
Compare
|
Suits me fine. I opened a follow-up for 6.4 and will do another for 7.1 for the remaining ones if this gets actually merged in 5.4 |
derrabus
approved these changes
Jul 16, 2024
8c14966 to
08404b2
Compare
derrabus
reviewed
Jul 18, 2024
src/Symfony/Component/HttpFoundation/Tests/RateLimiter/AbstractRequestRateLimiterTest.php
Outdated
Show resolved
Hide resolved
fabpot
added a commit
that referenced
this pull request
Jul 19, 2024
…yFileResponse` (alexandre-daubois) This PR was merged into the 7.2 branch. Discussion ---------- [HttpFoundation] Remove always false condition in `BinaryFileResponse` | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | no | Issues | - | License | MIT The condition is always false because the parameter is int typed. Passing `PHP_INT_MAX + 1` will result on a cast to float, making PHP complain about the parameter type. Also, the exception raised should better be an `InvalidArgmuentException` to me, instead of a logic one. I moved in this PR the test covering `setChunkSize` originally proposed in #57714. Commits ------- 13fca6a [HttpFoundation] Remove always false condition in `BinaryFileResponse`
08404b2 to
28c58c7
Compare
src/Symfony/Component/HttpFoundation/Tests/RateLimiter/AbstractRequestRateLimiterTest.php
Outdated
Show resolved
Hide resolved
28c58c7 to
d9fa800
Compare
|
Thank you @alexandre-daubois. |
nicolas-grekas
added a commit
that referenced
this pull request
Jul 26, 2024
…d `SchemeRequestMatcher` (alexandre-daubois) This PR was merged into the 6.4 branch. Discussion ---------- [HttpFoundation] Add tests for `MethodRequestMatcher` and `SchemeRequestMatcher` | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT Following #57714 after rebasing on 5.4 Commits ------- 5a3c2a6 [HttpFoundation] Add tests for `MethodRequestMatcher` and `SchemeRequestMatcher`
symfony-splitter
pushed a commit
to symfony/http-foundation
that referenced
this pull request
Jul 26, 2024
…d `SchemeRequestMatcher` (alexandre-daubois) This PR was merged into the 6.4 branch. Discussion ---------- [HttpFoundation] Add tests for `MethodRequestMatcher` and `SchemeRequestMatcher` | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT Following symfony/symfony#57714 after rebasing on 5.4 Commits ------- 5a3c2a63c7 [HttpFoundation] Add tests for `MethodRequestMatcher` and `SchemeRequestMatcher`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.