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

PhpRendererStrategy should be able to handle an event w/o a response #112

Conversation

MidnightDesign
Copy link
Contributor

ViewEvent#getResponse() may return null, but PhpRendererStrategy#injectResponse() didn't handle that case.

ViewEvent#getResponse() may return null, but PhpRendererStrategy#injectResponse() didn't handle that case.
@MidnightDesign
Copy link
Contributor Author

I wasn't sure what to test for since I just expect the method not to throw an error. Please let me know if I should change the test.

@MidnightDesign
Copy link
Contributor Author

What should I do about the PHPUnit incompatibility? (expectException()/setExpectedException())

@froschdesign
Copy link
Member

froschdesign commented Mar 20, 2017

@MidnightDesign

What should I do about the PHPUnit incompatibility?

At the moment zend-view still using PHPUnit 4.5.

@MidnightDesign
Copy link
Contributor Author

@froschdesign So I shouldn't do anything and the failing builds are OK?

@weierophinney weierophinney added this to the 2.8.2 milestone Mar 20, 2017
@weierophinney weierophinney self-assigned this Mar 20, 2017
@weierophinney
Copy link
Member

@MidnightDesign I've merged other commits today that upgrade the PHPUnit install; as long as your new test passes, it's fine.

@weierophinney weierophinney merged commit 239eef9 into zendframework:master Mar 20, 2017
weierophinney added a commit that referenced this pull request Mar 20, 2017
…response

PhpRendererStrategy should be able to handle an event w/o a response
weierophinney added a commit that referenced this pull request Mar 20, 2017
weierophinney added a commit that referenced this pull request Mar 20, 2017
weierophinney added a commit that referenced this pull request Mar 20, 2017
@weierophinney
Copy link
Member

Thanks, @MidnightDesign

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

Successfully merging this pull request may close these issues.

3 participants