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

Add TesterTrait::assertCommandIsSuccessful() helper#41851

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

Merged

Conversation

@yoannrenard
Copy link
Contributor

QA
Branch?5.4
Bug fix?no
New feature?yes
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

This PR introduces a new helper TesterTrait::assertCommandIsSuccessful() that aims to help testing the result of a command.
This is inspired byBrowserKitAssertionsTrait::assertResponseIsSuccessful

ro0NL reacted with thumbs up emoji
@carsonbot
Copy link

Hey!

I think@Jean85 has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@yoannrenard
Copy link
ContributorAuthor

It looks like tests on Travis with PHP 8.0 fail as they are run againstsymfony/console tagsv5.2.5/6.0.x-dev which both don't contain this new method added by this PR.
Shall I move those changes into another PR in order to make them pass?

@xabbuh
Copy link
Member

Thedeps=high build is allowed to fail in such a case. It will fix itself automatically when branches are merged up. Fordeps=low you need to modify thecomposer.json file of the component in question and modify version constraints in a way that no pre 5.4 release of the Console component will be installed.

@yoannrenardyoannrenardforce-pushed theadd_commandIsSuccesful_helper branch from0d18b9c todb39567CompareJune 26, 2021 10:42
@yoannrenard
Copy link
ContributorAuthor

Thanks for your help@xabbuh
I wonder, though, if this doesn't break the BC promise...

Copy link
Member

@chalasrchalasr 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 PR. I don't see any backward incompatible change here.
LGTM but the CHANGELOG file needs to be updated

@yoannrenard
Copy link
ContributorAuthor

Updated@chalasr

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.

rebase needed

5.4
---

* Add`TesterTrait::assertCommandIsSuccessful()` to test command

Choose a reason for hiding this comment

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

missing space at start of line

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Good catch

"symfony/expression-language":"^4.4|^5.0|^6.0",
"symfony/config":"^4.4|^5.0|^6.0",
"symfony/console":"^4.4|^5.0|^6.0",
"symfony/console":"^5.4|^6.0",

Choose a reason for hiding this comment

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

the conflict line should also be updated

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

done

"symfony/asset":"^5.3|^6.0",
"symfony/browser-kit":"^4.4|^5.0|^6.0",
"symfony/console":"^5.2|^6.0",
"symfony/console":"^5.4|^6.0",

Choose a reason for hiding this comment

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

the conflict line should be updated too

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

done

@nicolas-grekasnicolas-grekas added this to the5.4 milestoneJul 2, 2021
@yoannrenardyoannrenardforce-pushed theadd_commandIsSuccesful_helper branch 2 times, most recently from4d7badc to434e3d2CompareJuly 2, 2021 12:22
@chalasrchalasrforce-pushed theadd_commandIsSuccesful_helper branch from434e3d2 to6221527CompareJuly 3, 2021 17:16
@chalasr
Copy link
Member

Thank you@yoannrenard.

@chalasrchalasr merged commitcdcf696 intosymfony:5.4Jul 3, 2021
@yoannrenardyoannrenard deleted the add_commandIsSuccesful_helper branchJuly 4, 2021 06:48
This was referencedNov 5, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@derrabusderrabusderrabus approved these changes

@chalasrchalasrchalasr approved these changes

@srozesrozeAwaiting requested review from sroze

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

6 participants

@yoannrenard@carsonbot@xabbuh@chalasr@nicolas-grekas@derrabus

[8]ページ先頭

©2009-2025 Movatter.jp