[Console] Add a stream helper #53518
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| $commandTester->setInputs([$input]); | ||
|
|
||
| $commandTester->execute(['--format' => 'html']); |
There was a problem hiding this comment.
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 to
8bb6db9
Compare
OskarStark
reviewed
Jan 12, 2024
8bb6db9 to
2cb735c
Compare
|
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? |
nicolas-grekas
added a commit
that referenced
this pull request
Jan 29, 2024
…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 | MIT Fix 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 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As I explain in another 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 ?