spotlight icon indicating copy to clipboard operation
spotlight copied to clipboard

Adds basic test suite for existing functionality

Open ksassnowski opened this issue 4 years ago • 4 comments

This PR adds a basic test suite for the existing functionality. Since I'm not too familiar with Livewire, I didn't add any tests using Livewire::test(). I also tried to mostly document the existing behavior without changing too much of the implementation.

There were a few issues I noticed while writing the tests:

  • The execute method on the Spotlight class does not check if getCommandById actually found a command. If it returns null the method_exists check will fail with a TypeError.
  • The placeholder on SpotlightCommandDependency doesn't have a default value but also isn't required in the constructor. This effectively makes it mandatory to call setPlaceholder since you'd run into an error when calling toArray because the $placeholder property has not been initialized yet.
  • The setType method on the SpotlightCommandDependency class accepts any string, even though it looks like the package only knows how to deal with search and input. Maybe replace it with two separate methods to set the $type to search or input, respectively?

Anyways, let me know what you think!

ksassnowski avatar Apr 23 '21 19:04 ksassnowski

Thanks @ksassnowski - Really appreciate the time you took to make improvements and write the tests🙌 I'll take a look this weekend or Monday and get back to you! Have a great weekend!

PhiloNL avatar Apr 23 '21 20:04 PhiloNL

Sorry for the wait @ksassnowski, it's still on my list to review, thanks for your patience 🙏

PhiloNL avatar Apr 29 '21 14:04 PhiloNL

No worries 🤘

ksassnowski avatar Apr 29 '21 17:04 ksassnowski

Updates?

Krato avatar Aug 08 '22 08:08 Krato