Skip to content

Conversation

@jsor
Copy link
Member

@jsor jsor commented Oct 11, 2017

Copy link
Member

@WyriHaximus WyriHaximus left a comment

Choose a reason for hiding this comment

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

LGTM 👍

}

return $mock;
}
Copy link
Member

Choose a reason for hiding this comment

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

How would we test this method? 😋

On a more serious note, this looks really complicated for such a simple test. As an alternative, maybe keep a counter somewhere and explicitly call the handler twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a copy of the expectCallableConsecutive() from above with additional $mock->willReturnCallback() ;)

As an alternative, maybe keep a counter somewhere and explicitly call the handler twice?

Not sure what you mean here? Could you elaborate?

Copy link
Member Author

@jsor jsor Oct 13, 2017

Choose a reason for hiding this comment

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

@clue I've updated the test and removed the test helper.

@jsor jsor force-pushed the middlewarerunner-immutable-stack-test branch from 7fbc299 to 6c846ec Compare October 13, 2017 07:06
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.

Thanks for the update @jsor, changes LGTM! 👍

@WyriHaximus WyriHaximus merged commit 6f7b324 into reactphp:master Oct 13, 2017
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