mikebronner / laravel-model-caching

Eloquent model-caching made easy.
MIT License
2.24k stars 214 forks source link

Add nova tests #327

Closed dmason30 closed 4 years ago

dmason30 commented 4 years ago

Fixes #283, #278, #280

I created this from add-nova-tests branch and then merged in master to make sure everything is up to date.

I changed the nova support to 3.x as that is the laravel 7 compatible version.

This should work for you locally as an initial setup for nova I feel that either performing all actions via the api or by accessing the nova related code directly would be the best approach.

The test is still just a placeholder for whatever actual tests need to be done.

mikebronner commented 4 years ago

Are you submitting a new PR for this? :) Just curious why you closed it. Was going to look into it more in the near future.

dmason30 commented 4 years ago

@mikebronner Sorry, I accidentally closed it ha I have just added some initial tests for CUD and BelongsToMany and I didn't come across anything broken but maybe I am missing something?

Now pointing at master btw

mikebronner commented 4 years ago

After merging this, I am getting the following error when trying to run PHPUNIT:

1) GeneaLabs\LaravelModelCaching\Tests\Feature\Nova\BelongsToManyTest::testAttachRelationFlushesCache
Expected status code 200 but received 500.
Failed asserting that 200 is identical to 500.

/Users/mike/Developer/Sites/laravel-model-caching/vendor/laravel/framework/src/Illuminate/Testing/TestResponse.php:185
/Users/mike/Developer/Sites/laravel-model-caching/tests/Feature/Nova/BelongsToManyTest.php:24

The error in the test response appears to be:

SQLSTATE[HY000]: General error: 1 no such table: action_events

However, the nova migrations are included in the migration folder, so not sure what's happening? How were you able to get the tests to pass?

dmason30 commented 4 years ago

@mikebronner I had some issues with this and found that removing and creating a blank baseline.sqlite then running full suite seemed to the trick. Although, I thought committing the sqlite files would mean the table would already be migrated.

I have just checked out master and it is working as expected. I think there is something up with the way the tests are handling migrations.

To be honest i find the setup quite confusing I understand the aim is to speed up tests but I feel neither of the sqlite files should be committed. I would remove the AlwaysRunFirstTest and instead create and migrate baseline.sqlite in a base TestCase once per run (self::$migrated = true) and keep the copying to testing.sqlite. Then gitignore the sqlite files.

Running single tests would be slower but at least you know you are using a fresh sqlite db every run.

mikebronner commented 4 years ago

Hi @dmason30 -- the sqlite files probably don't need to be versioned, you are right about that. And they are recreated when the tests are run. The problem with moving it to a base TestCase is that it is then recreated once for every class. Moving it out to a test that is run before all other tests was the only way to make this work only once per run.

I was running the whole suite when I got the errors above, but didn't have more time to investigate.

dmason30 commented 4 years ago

@mikebronner You can still make it run once with a TestCase just use wrap it in a static flag hence I mentioned the self::$migrated = true. I'll see if I have time to create a PR with a proof of concept for you.