Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[BrowserKit] Add jsonRequest function to the browser-kit client#38596

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@alexander-schranz
Copy link
Contributor

@alexander-schranzalexander-schranz commentedOct 16, 2020•
edited
Loading

QA
Branch?5.x
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix -
LicenseMIT
Doc PRsymfony/symfony-docs#...

If you use the FOSRestBundle for your Api's you have maybe many tests using just:

$client->request('POST','/api/contacts', ['param' =>1]);

To test your JSON api as in the real browser request FOSRestBundle converts the json body into the$request->request object. I think something similar is done by ApiPlatform. If you have tests like above they will now fail as the integer is converted to string see also#38591.

This PR add a newjsonRequest which will look like the following and will so fix the above problem:

$client->jsonRequest('POST','/api/contacts', ['param' =>1]);

@alexander-schranzalexander-schranzforce-pushed thefeature/browser-kit-json-request branch 2 times, most recently frome57df9b to4fdf2faCompareOctober 16, 2020 09:45
@chalasrchalasr added this to the5.x milestoneOct 16, 2020
@alexander-schranzalexander-schranzforce-pushed thefeature/browser-kit-json-request branch 2 times, most recently from4fdf2fa toe2edfc1CompareOctober 16, 2020 09:59
@alexander-schranzalexander-schranzforce-pushed thefeature/browser-kit-json-request branch fromcc1415f to696a3f8CompareOctober 16, 2020 10:21
@Nyholm
Copy link
Member

Thank you

I like the idea of this because I know there are a lot of parameters to set when you want to test some custom requests. I am not sure it is a good idea to hard code the content type toapplication/json. I build all my APIs withJson:API format which requires a content type ofapplication/vnd.api+json. So I cannot leverage this feature in the current state.

Just FYI. There is asmall package from@nebkam that I like. It has aRequestBuilder to solve the same problem as this PR is trying to solve.

Copy link
Member

@ycerutoyceruto left a comment

Choose a reason for hiding this comment

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

LGTM.

@Nyholm about to hard code the content type toapplication/json I think it's fine as default value. Anyway, you'll can change these server parameters through the$server argument if you want.

$server =array_merge($this->server,$server);

@nicolas-grekasnicolas-grekas changed the titleAdd jsonRequest function to the browser-kit client[BrowserKit] Add jsonRequest function to the browser-kit clientOct 19, 2020
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks. Don't miss adding a changelog entry in the component.

@alexander-schranz
Copy link
ContributorAuthor

@nicolas-grekas changelog is updated.

fabbot is failing because of#38596 (comment) and#38596 (comment).

Let me now if I need to change something.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

What about@Nyholm's comment?
(yes you can ignore fabbot here, the report is a false-positive)

@alexander-schranz
Copy link
ContributorAuthor

alexander-schranz commentedOct 20, 2020•
edited
Loading

@Nyholm I think we should go here withapplication/json as we do the same inJsonResponse which sets there theContent-Type toapplication/json and I think this both should match.

@alexander-schranzalexander-schranzforce-pushed thefeature/browser-kit-json-request branch 2 times, most recently from4948af9 tob1123d2CompareOctober 20, 2020 12:30
@alexander-schranzalexander-schranzforce-pushed thefeature/browser-kit-json-request branch fromb1123d2 to596b2c0CompareOctober 20, 2020 12:36
derrabus added a commit that referenced this pull requestOct 20, 2020
…chranz)This PR was merged into the 4.4 branch.Discussion----------Refractor AbstractBrowserTest to assertSame| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| Deprecations? | no| Tickets       | Fix #...instead -->| License       | MIT| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->As mentioned by@nicolas-grekas at#38596 (comment) the AbstractBrowserTest should be refractored to use `assertSame` instead of `assertEquals` to compare string.Commits-------3f320c8 Refractor AbstractBrowserTest to assertSame
}

/**
* Converts the request parameters into a json string and used it as request content.

Choose a reason for hiding this comment

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

json string ->JSON string

and used it ->and uses it

@chalasrchalasrforce-pushed thefeature/browser-kit-json-request branch from52efab2 toc2fa2cbCompareNovember 11, 2020 13:22
@chalasr
Copy link
Member

Thank you@alexander-schranz.

@chalasrchalasr merged commit16fb94b intosymfony:5.xNov 11, 2020
@alexander-schranzalexander-schranz deleted the feature/browser-kit-json-request branchNovember 11, 2020 13:24
Copy link

@beongohl997beongohl997 left a comment

Choose a reason for hiding this comment

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

A

Copy link

@beongohl997beongohl997 left a comment

Choose a reason for hiding this comment

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

src/Symfony/Component/BrowserKit/AbstractBrowser.php

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@stofstofstof left review comments

@jderussejderussejderusse left review comments

@derrabusderrabusderrabus left review comments

@javiereguiluzjaviereguiluzjaviereguiluz approved these changes

@ycerutoycerutoyceruto approved these changes

@chalasrchalasrchalasr approved these changes

+1 more reviewer

@beongohl997beongohl997beongohl997 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

11 participants

@alexander-schranz@Nyholm@chalasr@javiereguiluz@nicolas-grekas@stof@jderusse@derrabus@yceruto@beongohl997@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp