Add a watchdog tail command #4767
Conversation
| // TODO: how should the output string be built? | ||
| // Just printing stuff to show it doing something. | ||
| $row = $this->formatResult($result, $options['extended']); | ||
| print "\n " . $row->wid . "\t"; |
There was a problem hiding this comment.
I think $this->writeln($string) would be appropriate instead of print.
There was a problem hiding this comment.
ack. @greg-1-anderson gave me some pointers re. output in Slack. one idea was to create another command for the tail case. thoughts?
There was a problem hiding this comment.
It's super weird when a command returns RowsOfFields sometimes, and nothing (or some completely different type) at other times, but since --tail never returns at all, I guess it's not so bad that the behavior is inconsistent.
I'm trying to deprecated $this->io() and $this->writeln() et. al. in Robo; the better way is to add an OutputInterface as the first parameter. Then you'll get an object you can writeln() on.
There was a problem hiding this comment.
I've update the PR to use an injected OutputInterface object.
| * @param string $input | ||
| * A string representing the STDIN that is piped to the command. | ||
| */ | ||
| public function startExecute($command, $cd = null, $env = null, $input = null) |
There was a problem hiding this comment.
Is there no way to use $this->execute()? Would save a lot of duplicated code.
There was a problem hiding this comment.
there is a lot of duplicated code, and i'm open to ways to reduce it.
the main new capability needed is 'run child process in the background, without blocking on it's completion, with access to output for tests'.
my first thought was that should be a separate method, rather than another flag and return type from execute(). so i was planning to reduce duplicate code by factoring out the shared bits in execute() and startExecute(). i also don't love the name startExecute(), happy to get better suggestions.
make sense?
There was a problem hiding this comment.
How about we sadd a $timeout param and stop hardocding 0 as timeout in this line. And we add the same timeout param to drush() method which thencalls execute(). Then the test can just call something like $this->drush(watchdog:show --tail, $timeout)
There was a problem hiding this comment.
I think that's a useful but slightly different improvement.
To feel confident that this works, I'd like to create multiple watchdog entries while the process running watchdog:show --tail is running. That requires interactivity in the test process, so we can't block on the $this->drush() call.
There was a problem hiding this comment.
updated the PR to factor out much of the duplicated code.
beejeebus
changed the title
|
I get the same failures on master locally as in CI. Could use some pointers for figuring out the tests. |
|
The test failure is unrelated and is being fixed elsewhere. |
|
Is this available in any release yet, and if so, which versions? |
|
Its available since 10.6. FYI this info is now viewable on the command's doc page https://www.drush.org/latest/commands/watchdog_tail/ |
WIP, feedback welcome.
Only tested with
composer functional -- --filter testWatchdogTail.