Skip to content

[Console] Add support for Invokable Commands in CommandTester #60823

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.

Sign up for GitHub

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

Jump to bottom
Merged
fabpot merged 1 commit into from
Jun 23, 2025
Merged

[Console] Add support for Invokable Commands in CommandTester #60823

fabpot merged 1 commit into from
Jun 23, 2025

Conversation

@ruudk
Copy link
Contributor

@ruudk ruudk commented Jun 18, 2025 •
edited
Loading

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Issues
License MIT

I'm trying out the amazing new Invokable Commands introduced in Symfony 7.3 but I noticed that I could use it together with the CommandTester. It was also not possible to manually add these type of commands to the Application.

I'm not sure what the best approach is with this. As I'm changing the signature of the methods this might be considered a breaking change. Or it might not, as I'm widening the type from Command to object.

Please advise what the best approach would be to introduce something like this. Thanks 🙏

/cc @yceruto @chalasr

@ruudk ruudk requested a review from chalasr as a code owner June 18, 2025 12:45
@carsonbot carsonbot added this to the 7.3 milestone Jun 18, 2025
@ruudk ruudk changed the title [Console] Invokable commands [Console] Add support for Invokable Commands in CommandTester and Application Jun 18, 2025
@ruudk ruudk changed the title [Console] Add support for Invokable Commands in CommandTester and Application [Console] Add support for Invokable Commands in CommandTester and Application Jun 18, 2025
@ruudk
Copy link
Contributor Author

ruudk commented Jun 18, 2025

Sure I could wrap them manually in new Command()->setCode($theCommand) but that feels nasty from a user perspective.

@GromNaN GromNaN changed the base branch from 7.3 to 7.4 June 18, 2025 13:32
@GromNaN
Copy link
Member

GromNaN commented Jun 18, 2025

I updated the target branch to 7.4, this is a new feature that cannot be merged into 7.3

@ruudk ruudk force-pushed the invokable-commands branch 2 times, most recently from 2e104a7 to beea196 Compare June 18, 2025 13:42
@ruudk
Copy link
Contributor Author

ruudk commented Jun 18, 2025

Thanks, it seems that the Application was already fixed in 7.4 #60394

@ruudk ruudk changed the title [Console] Add support for Invokable Commands in CommandTester and Application [Console] Add support for Invokable Commands in CommandTester Jun 18, 2025
GromNaN
GromNaN requested changes Jun 18, 2025
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

That's good, there are a few things to fix to clean up the PR.
Thanks for the new test.

@ruudk ruudk force-pushed the invokable-commands branch from beea196 to 74ca920 Compare June 18, 2025 13:54
@ruudk ruudk requested a review from GromNaN June 18, 2025 13:56
@ruudk
Copy link
Contributor Author

ruudk commented Jun 18, 2025 •
edited
Loading

@GromNaN Done, also updated the changelog!

yceruto
yceruto reviewed Jun 18, 2025
View reviewed changes
yceruto
yceruto reviewed Jun 18, 2025
View reviewed changes
@ruudk ruudk force-pushed the invokable-commands branch from 47ff4cc to e1d9dce Compare June 19, 2025 08:15
@GromNaN GromNaN self-requested a review June 19, 2025 08:54
GromNaN
GromNaN reviewed Jun 19, 2025
View reviewed changes
GromNaN
GromNaN reviewed Jun 19, 2025
View reviewed changes
GromNaN
GromNaN reviewed Jun 19, 2025
View reviewed changes
GromNaN
GromNaN reviewed Jun 19, 2025
View reviewed changes
GromNaN
GromNaN requested changes Jun 19, 2025
View reviewed changes
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

The test needs to be modified, and then we're all good!

@ruudk
Copy link
Contributor Author

ruudk commented Jun 19, 2025

@GromNaN Done, I also added another commit with extra tests.

@GromNaN GromNaN added Feature and removed Bug labels Jun 19, 2025
@ruudk
Copy link
Contributor Author

ruudk commented Jun 19, 2025

@GromNaN @yceruto Thanks for the thorough reviews and guidance on this 👏

yceruto
yceruto approved these changes Jun 19, 2025
View reviewed changes
Copy link
Member

@yceruto yceruto left a comment

Choose a reason for hiding this comment

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

Thanks @ruudk for the improvement!

stof
stof reviewed Jun 19, 2025
View reviewed changes
HypeMC
HypeMC reviewed Jun 19, 2025
View reviewed changes
@ruudk ruudk force-pushed the invokable-commands branch from 40a5a5f to a10ae7f Compare June 20, 2025 07:25
@fabpot fabpot modified the milestones: 7.3, 7.4 Jun 23, 2025
@fabpot
Copy link
Member

fabpot commented Jun 23, 2025

Thank you @ruudk.

@fabpot fabpot merged commit 138109a into symfony:7.4 Jun 23, 2025
10 of 11 checks passed
@fabpot fabpot mentioned this pull request Jun 23, 2025
Closed
@HypeMC HypeMC mentioned this pull request Jun 24, 2025
Merged
chalasr added a commit that referenced this pull request Jun 24, 2025
@chalasr
minor #60884 [Console] Cleanup test for invokable command (HypeMC)
fa29d30 
This PR was merged into the 7.4 branch.

Discussion
----------

[Console] Cleanup test for invokable command

| Q             | A
| ------------- | ---
| Branch?       | 7.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

A slight cleanup after #60823.

Commits
-------

df467e2 [Console] Cleanup test
@GromNaN
Copy link
Member

GromNaN commented Jul 9, 2025

I got the question about testing invokable commands in Symfony 7.3. While this feature will be released in Symfony 7.4, here are two solutions:

  1. Create a Command object and inject the invokable command: You can achieve this by doing new CommandTester((new Command)->setCode($invokableCommand))).
  2. Directly test the __invoke method of your class: treat it like a unit test, using ArrayInput and BufferedOutput.

@chalasr
Copy link
Member

chalasr commented Jul 9, 2025

Third testing option for 7.3: use ApplicationTester, same capabilities as CommandTester

@dbu dbu mentioned this pull request Aug 20, 2025
Merged
This was referenced Oct 27, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stof stof stof left review comments

@HypeMC HypeMC HypeMC left review comments

@fabpot fabpot fabpot approved these changes

@GromNaN GromNaN GromNaN approved these changes

@yceruto yceruto yceruto approved these changes

@chalasr chalasr Awaiting requested review from chalasr chalasr is a code owner

Assignees

No one assigned

Labels

Console Feature Status: Reviewed

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

8 participants

@ruudk @GromNaN @fabpot @chalasr @stof @yceruto @HypeMC @carsonbot