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

Set the Content-Length header when emitting the response #88

Merged
merged 2 commits into from
Oct 15, 2015

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented Aug 20, 2015

Fixes #84

One test is failing because of a side-effect in unrelated tests. Need to work on it, but if you have an idea on the best way to fix it's welcome, I'm not familiar with everything yet.

->expects($this->any())
->method('withHeader')
->willReturnSelf();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really happy with that, mocking the whole world to get an integration test passing isn't really great. Didn't have a lot of time to think of a better solution.

Copy link
Member

Choose a reason for hiding this comment

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

With some of these integration tests, it seems like the only way to do it. They do get lengthy, though!

We might consider porting them to Prophecy at some point (we're using it extensively in the ZF3 component rewrites, and in Expressive), but that's something for a lull time in development.

@mnapoli
Copy link
Contributor Author

mnapoli commented Aug 31, 2015

The tests should be green now.

Please review the tests and let me know if you want me to apply a different solution: auto-adding the Content-Length header broke several tests assertions (unexpected Content-Length header appeared).

@weierophinney weierophinney added this to the 1.1.4 milestone Oct 15, 2015
@weierophinney weierophinney self-assigned this Oct 15, 2015
@weierophinney weierophinney modified the milestones: 1.2.0, 1.1.4 Oct 15, 2015
@weierophinney weierophinney removed the bug label Oct 15, 2015
@weierophinney weierophinney merged commit e0aff7a into zendframework:develop Oct 15, 2015
weierophinney added a commit that referenced this pull request Oct 15, 2015
Set the Content-Length header when emitting the response
weierophinney added a commit that referenced this pull request Oct 15, 2015
weierophinney added a commit that referenced this pull request Oct 15, 2015
@mnapoli mnapoli deleted the content-length branch October 16, 2015 06:37
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.

None yet

2 participants