wisemen-digital / devops-github-actions

GNU General Public License v3.0
0 stars 1 forks source link

Laravel: remove ` migrate --seed` from test step #2

Open PauwelsPieter opened 6 months ago

PauwelsPieter commented 6 months ago

Migration and seeder is executed before tests run.


On another note: in the archive the command tar -czf dist.tar.gz --exclude=.git* * .??* the .env.testing file isn't copied. This causes the APP_ENV (for example) not to be set and some conditions not to be met.

djbe commented 6 months ago

Good catch! Will look into it.

djbe commented 6 months ago

@PauwelsPieter to be clear, the actual issue is the missing env file, right? Because the title of this issue asks to remove migrate --seed 😕

djbe commented 6 months ago

Hmmm are you sure this is an issue? 'cause you can download the actual result of the build (of a PR Check), like this one: https://github.com/wisemen-digital/start-to-stop-api/actions/runs/8718392651

Extracting it, the .env.testing file is in there, so everything looks fine? To clarify the tar command:

PauwelsPieter commented 6 months ago

@PauwelsPieter to be clear, the actual issue is the missing env file, right? Because the title of this issue asks to remove migrate --seed 😕

Yea, better made 2 issues. The seed is unnecessary because the php artisan migrate:fresh --seed command is run once before all the tests, and the tests are run in transaction.

PauwelsPieter commented 6 months ago

Hmmm are you sure this is an issue? 'cause you can download the actual result of the build (of a PR Check), like this one: https://github.com/wisemen-digital/start-to-stop-api/actions/runs/8718392651

Extracting it, the .env.testing file is in there, so everything looks fine? To clarify the tar command:

  • It excludes .git*, so the .git folder as well as any .gitignore, .gitkeep and .gitattributes files
  • It adds all files & folders. Note that this does not include hidden files
  • It also adds all hidden files & folders, starting with at least a . and 2 characters. I think this was needed to avoid adding .. (the parent directory).

I see, the tar command is just to create the GH artifact. I just tested if the APP_ENV is filled in when dumped in my test code, and it does.

I assume I ran into this issue because the APP_ENV !== 'testing' was performed in the migration and because the migration is run before the .env.testing is copied to .env it would pass this check in the testing pipeline.

So in conclusion, the issue is okay. Make sure to remove the run migration command from the pipeline because we do this before running the tests. And otherwise if it would be necessary, make sure the .env is created before running this command.

djbe commented 6 months ago

That's the weird thing, .env exists and is correct BEFORE we run the migrations & seed. Here's the action code:

cp -f .env.testing .env
…
cp -f .env .env.testing
php artisan config:cache
php artisan migrate --seed

Note that we had to add the migration&seed step, because the tests were failing in some repositories without it.

PauwelsPieter commented 6 months ago

That's the weird thing, .env exists and is correct BEFORE we run the migrations & seed. Here's the action code:

Ehm, just tested it, and it works! Very strange why I runned into this issue at the first place 😕.

Note that we had to add the migration&seed step, because the tests were failing in some repositories without it.

Those repos have to be updated. I'll pick this up with circle Laravel.

djbe commented 6 months ago

Let me know, and if y'all agree on integrating this into the tests themselves, then we can safely remove the migration command from the action 👍