Optimize CommandRunner and Commands

[paulbalandan]

Checklist:

• Securely signed commits
• Component(s) with PHPdocs
• Unit testing, with >80% coverage
• Conforms to style guide

[paulbalandan]

@MGatner this optimization made all test cases of CommandRunnerTest to be all under 0.50 s.

[MGatner]

Glad you cracked this egg! Moving the discovery to the constructor could cause some people problems, but generally I think it is a better design. Maybe see if we can get another review from @lonnieezell or @samsonasik on that point? Otherwise just a couple of tweaks.

[lonnieezell]

I typically prefer not to do things like that in a constructor but in this case I don't see a problem with it. Commands aren't something that are called all of the time, and testing is where that has the tendency to be the biggest issue. But that's why we're moving it now so it looks good to me!

[paulbalandan]

Glad you cracked this egg! Moving the discovery to the constructor could cause some people problems, but generally I think it is a better design. Maybe see if we can get another review from @lonnieezell or @samsonasik on that point? Otherwise just a couple of tweaks.

I have pondered on this and I don't think moving discovery to the constructor would give us problems. Discovery essentially fills up the $commands array prop with the discovered commands. Current design shows that every invoke of run we call discoverCommands which in turn returns early if the $commands prop is already populated. So this prop is never overwritten. Moving the discovery at constructor initialization would be a big gain, as the tests demonstrate.