Skip to content

[FrameworkBundle] Add BrowserKitAssertionsTrait::assertThatForBrowser #42526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

koenreiniers
Copy link
Contributor

@koenreiniers koenreiniers commented Aug 13, 2021

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

I wanted to create a constraint that acts on the request and the response (an openapi schema validator constraint that needs the method and pathinfo from the request and the body from the response). But, there is currently no way to get the current client, since BrowserKitAssertionsTrait::getClient is private and there is no method to do an assertion on the client/browser (like BrowserKitAssertionsTrait::assertThatForResponse for the response).

This small change will allow to do the following:

protected static function assertResponseMatchesOpenApiSchema(): void
{
    self::assertThatForBrowser(new ResponseMatchesOpenApiSchema(self::getOpenApiValidator()));
}

@fabpot
Copy link
Member

fabpot commented Aug 26, 2021

When you are calling your assertion in a test, you do have access to the current client, so I don't see the benefit of this new method (which makes difficult to understand what's going on IMHO).

@nep-koen
Copy link

Yes I agree that the benefits are small, though someone at some points made the decision to be able to call browser kit assertions without passing any arguments, instead acting on the internal state of the test class (e.g. self::assertResponseIsSuccessful(), self::assertBrowserHasCookie(..), etc.).

For the sake of consistency I would like to be able to do the same for my own assertions (e.g. self::assertResponseMatchesOpenApiSchema() instead of self::assertResponseMatchesOpenApiSchema($client) or self::assertResponseMatchesOpenApiSchema($method, $path).

If I hadn't needed the request method and path this would have been completely possible using BrowserKitAssertions::assertThatForResponse(), which does the same thing for the Response object as the new BrowserKitAssertions::assertThatForBrowser() method does for the AbstractBrowser.

Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Ok, fair enough, that makes sense to me. Can you rename the method to be consistent with the current naming?

@fabpot fabpot force-pushed the assert-for-browser branch from 67b6b0b to b2c32ea Compare August 26, 2021 07:10
@fabpot
Copy link
Member

fabpot commented Aug 26, 2021

Thank you @koenreiniers.

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.

5 participants