search

zenstruck / foundry

A model factory library for creating expressive, auto-completable, on-demand dev/test fixtures with Symfony and Doctrine.
https://symfony.com/bundles/ZenstruckFoundryBundle/current/index.html
MIT License
609 stars 63 forks source link

chore: remove Makefile #513

Closed nikophil closed 7 months ago

kbond commented 8 months ago

Some thoughts:

nikophil commented 8 months ago

The non-dama tests we run on every permutation that includes a database is really slow... wondering how we can speed this up, only run on a single permutation? Maybe the code coverage job?

ok, this is a simple solution to not mess with a more complex matrix, and even test one permutation with dama. Done ✅

We're no longer testing with migrations, this would need the auto-generate migrations before the test system I have in foundry-next

migrations are tested in ORMDatabaseResetterTest and WithMigrationTest

By the way, I was surprised that in foundry-next, you were running two whole test suites on all permutation in order to test migration, is it really needed? :thinking:

The reset mode does not change the whole behavior of foundry, I don't think it deserves all this CI time!?

nikophil commented 8 months ago

Hey!

I think this PR is almost ready (minus your future comments :sweat_smile:)

About the migrations

I think having ONE test (ie: WithMigrationTest) which uses ResetDatabase with migrations should be OK. One other test is also directly testing the reset database behavior: ORMDatabaseResetterTest

I've used your technique which uses bin/console and which generates automatically the migrations, I think it is pretty cool. In order not to generate a migration each time tests are started locally, I've conditioned it with an env var TEST_MIGRATIONS which I've enabled in the CI The migration works as well with mysql or postgres.

About Dama

I re-added it in the matrix, I think it is important to run the tests without dama at least once with postgre and once with mysql. Moreover ORMDatabaseResetterTest cannot run with dama and we want to test this. I've also used dama in the test coverage job in order to speed it up a little bit

I think code coverage fails because we're computing it with Foundry's bundle deactivated :shrug:

kbond commented 7 months ago

I'm good to go here.

I think code coverage fails because we're computing it with Foundry's bundle deactivated 🤷

Should the coverage job run the test suite 3 times (capturing/uploading a separate coverage.xml for each):

  1. dama
  2. no-dama
  3. no-bundle