Skip to content
This repository has been archived by the owner on Sep 15, 2022. It is now read-only.

Add fix for theResponseShouldContainsHeaders #141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

krizon
Copy link
Contributor

@krizon krizon commented Mar 24, 2015

Implemented proposed solution by @victuxbb in #118. I would also like to add specs but since the entire ApiContext is lacking specs and the theResponseShouldContainsHeaders method doesn't work at all at the moment I propose to merge the solution provided by @victuxbb in #118 and worry about specs later.

@wysow
Copy link
Contributor

wysow commented Aug 11, 2016

Hey @PedroTroller what about this one ? Really would like to see it merged and tagged ;) And BTW cheers to all the KNP Team!

@docteurklein
Copy link

I don't think it's a good idea to modify this step definition. You should create a new step named the headers should equal to.

This PR modifies the assertion from shlouldContain to shouldEqual, which I think is a bad idea.

@@ -198,7 +198,7 @@ public function iShouldReceiveResponse($httpCode, $shortType = null)
));
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This space is a mistake, I think it's your IDE which added it 😉

@wysow
Copy link
Contributor

wysow commented Aug 12, 2016

@docteurklein sure but the default definition just do not work at all (since when can't tell)... so better to make it work even with a different assertion no?

@wysow
Copy link
Contributor

wysow commented Aug 12, 2016

Error is:

Type error: Argument 2 passed to Knp\FriendlyContexts\Utils\Asserter::assertArrayContains() must be of the type array, object given, called in vendor/knplabs/friendly-contexts/src/Knp/FriendlyContexts/Context/ApiContext.php on line 214 (Behat\Testwork\Call\Exception\FatalThrowableError)

@docteurklein
Copy link

I would preferably fix the existing definition AND create another one for another assertion.

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

Successfully merging this pull request may close these issues.

4 participants