Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Console] Add a stream helper#53518
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| $commandTester->setInputs([$input]); | ||
| $commandTester->execute(['--format' => 'html']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
For this test, I can only use html because value of the dump doesn't be available in the command result. I seems that it be prompt in php stream not in the command.
@ogizanagi Could you help me to add cli format in this test ?
0131c92 to8bb6db9CompareUh oh!
There was an error while loading.Please reload this page.
8bb6db9 to2cb735cComparenicolas-grekas commentedJan 29, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for the PR.
It'd be fine making VarDumper a required dependency of the command in 7.1 (throwing a LogicException when DumpServer is not found in the constructor of ServerLogCommand) WDYT? |
…ouismariegaborit)This PR was merged into the 6.3 branch.Discussion----------[MonologBridge] Fix context data and display extra data| Q | A| ------------- | ---| Branch? | 6.3| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues | ->| License | MITFix the compatibility with Monolog 3. In the PR#52222 we had the compatibility with Monolog 3 but context parameter is incorrect and extra parameter is missing.No test can be added because testing this command is not possible at this time. Another PR (#53518) was opened to add test on this command.Commits-------8687ae0 Fix context data and display extra data
I'm closing for now, waiting for a new PR in case you're up to giving my above proposal a try 🙏 |
As I explain inanother PR, testing commands like
server:dumporserver:logis very difficult due to the infinite loop that waits for cancellation by the user.I suggest to create a new helper in console component to use same code between above two commands and test them to avoid regression in these commands.
What do you think ?
Regarding the use of the new StreamHelper, the Dumper component seems to no longer need the DumpServer class because we use now only the getHost function. To replace DumpServer in the command, I would like to use the ParameterBag but I can use it in the binary var-dump-server ? Or I should add a string parameter that I set by CompilerPass for the command ?