Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

WIP PSR7 conformance tests #49

Closed

Conversation

Maks3w
Copy link
Member

@Maks3w Maks3w commented Jun 8, 2015

This is an example of integration with maks3w/psr7-assertions a PSR-7 conformance test library

This library aims to provide a common set of tests for every PSR-7 implementation

@Maks3w Maks3w force-pushed the feature/psr7-compliance-suite branch from 353516b to ffe80ef Compare June 8, 2015 13:31
Already tested by PSR7 test suite compliance
> You can opt-in to preserving the original state of the Host header by passing true for the second ($preserveHost) argument. When this argument is set to true, the returned request will not update the Host header of the returned message -- unless the message contains no Host header.

**unless the message contains no Host header.** So $preserveHost MUST be ignored if message does not have a Host header
weierophinney added a commit to weierophinney/zend-diactoros that referenced this pull request Jun 12, 2015
weierophinney added a commit to weierophinney/zend-diactoros that referenced this pull request Jun 12, 2015
weierophinney added a commit to weierophinney/zend-diactoros that referenced this pull request Jun 12, 2015
- If none is provided, lazy-instantiate one.
@weierophinney
Copy link
Member

@Maks3w I've pushed fixes to issues to https://github.com/weierophinney/zend-diactoros/tree/hotfix/49

However, there's one test I disagree with: PHPUnit\ResponseInterfaceTestsTrait::testValidDefaultReasonPhrase. It assumes that if no reason phrase was provided, that an empty string must be returned. This is not true. The docblock explicitly says that implementations:

MAY choose to return the default RFC 7231 recommended reason phrase (or those listed in the IANA HTTP Status Code Registry) for the response's status code.

which is exactly what Diactoros does.

Once you fix that test, I think I can merge the changes both you and I have done.

@weierophinney weierophinney added this to the 1.0.4 milestone Jun 12, 2015
@weierophinney weierophinney self-assigned this Jun 12, 2015
@samsonasik
Copy link
Contributor

need rebase

@weierophinney
Copy link
Member

I've deleted several comments on this thread, as the behavior and communication were not civil or advancing the discussion of this pull request.

@Maks3w
Copy link
Member Author

Maks3w commented Jun 15, 2015

This PR is not intended to be merged. This PR only presents the Prs7Assertions project for to discuss about if the implementation is or not correct.

@weierophinney
Copy link
Member

This PR only presents the Prs7Assertions project for to discuss about if the implementation is or not correct.

@Maks3w Okay. That said, please see my note above: one of the assertions is incorrect currently. Implementations are allowed to cast a reason phrase to the IANA recommended reason phrase if one exists. If you truly want to check against an empty-string reason phrase, you'll need to set the status to one that isn't currently defined by the IANA.

@weierophinney
Copy link
Member

@Maks3w Also, while you may not intend to merge it, I fixed items as a result of merging locally. As such, I'd really love to have this mergeable at some point...

weierophinney added a commit that referenced this pull request Jun 22, 2015
- If none is provided, lazy-instantiate one.
weierophinney added a commit that referenced this pull request Jun 22, 2015
weierophinney added a commit that referenced this pull request Jun 22, 2015
@weierophinney
Copy link
Member

I've cherry-picked the code changes, and updated the two tests in RequestTest that needed changes to properly reflect the spec.

@Maks3w Maks3w reopened this Jun 23, 2015
@weierophinney weierophinney removed this from the 1.0.4 milestone Jun 24, 2015
@weierophinney
Copy link
Member

Closing due to lack of activity.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants