Add testing boilerplate, do not attempt to resolve abstract models by default

nonetallt commented 1 year ago

Add proper testing setup for the package.

Impact of the changes to the end-user should be minimal. Only change to the command is the addition of --resolve-abstract option that is now no longer enabled by default.

I decided against simply filtering abstract models since the ShowModel command attempts to resolve models from the application container, which means that it's possible for the user to have an abstract model bound to an instance in their container.

Changes:

Add testing setup
Add laravel-skeleton for testing
Add test watcher
Fix requirements and prune unused testing packages
Do not attempt to resolve abstract models by default (fixed #41)
Extract Generator::getModels() into a separate action GetModels
Improved model file name resolution supports fully qualified class names.
GetModels has an option to include or exclude models, but the option is currently unused.
Properly handle errors from the ShowModelCommand

Run tests with composer test or composer test-watch. I've added a couple of basic test as examples and to make sure that things work in general. Hopefully someone else can write more throughout coverage for the actual command output. Feel free to setup CI testing.

Problems:

Unfortunately the way Laravel handles output from nested commands makes debugging command output in the tests very obnoxious because Artisan::output() does not work after it's been called inside a command. This happens regardless of whether or not $mockConsoleOutput is being used. Note that this does not affect the actual output of the command in a live environment nor does it affect the output expectations of tests. You simply can't see the command output by calling Artisan::output() in the test after calling the model:typer command.

The alternative would be to pass the command to the underlying action and then use $command->call() instead of Artisan::call() but that doesn't work for the ShowModel command because we need the output immediately (and calling another command using a command routes the output into the output of that command).

Suggestions:

It might be a good idea to add a new --output-file option that will direct the output of the command to a specified file rather than printing directly to console output. This can then be used for easier test debugging as well as general use for the end-user. I already added a trait that can clean the output between tests.

tcampbPPU commented 1 year ago

Thanks for the PR @nonetallt this was much needed! Let me take a look over things and provide any feedback if needed.

acidjazz commented 1 year ago

@nonetallt can you by chance throw an action in here that kicks these tests off?

Here is an example of one https://github.com/fumeapp/laranuxt/blob/main/.github/workflows/test-php.yml

you can kill the branches/paths part etc

acidjazz commented 1 year ago

or better yet https://github.com/fumeapp/humble/blob/master/.github/workflows/test.yml

tcampbPPU commented 1 year ago

@nonetallt dont worry about the GH action stuff I will take care of it, there was a few extra I wanted to add

tcampbPPU commented 1 year ago

@nonetallt alright there are 2 composer stan issues left, pretty sure this suggestion will fix that. For the other one I am not sure, but I think we can ignore.

Thanks again for helping us out :)

nonetallt commented 1 year ago

I removed the following composer hook script:

{
  "scripts": {
    "post-autoload-dump": [
      "@php vendor/bin/testbench package:discover --ansi"
    ]
  }
}

Running the command is not required for testing because of the copied laravel-skeleton in test/laravel-skeleton. The command creates a symlink between the package vendor and testbench that can be problematic for local testing. See https://github.com/laravel/vite-plugin/issues/210 for more details.

fumeapp / modeltyper