dwyl / mvp

πŸ“² simplest version of the @dwyl app
https://mvp.fly.dev
GNU General Public License v2.0
84 stars 2 forks source link

[PR] Fix CI API tests failing #314

Closed LuchoTurtle closed 1 year ago

LuchoTurtle commented 1 year ago

closes #297 closes #315

codecov[bot] commented 1 year ago

Codecov Report

Merging #314 (999ee62) into main (cd0b132) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##              main      #314   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          475       475           
=========================================
  Hits           475       475           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

LuchoTurtle commented 1 year ago

This is super weird. The server is running and I've tested the tests locally and they all pass. But it seems that the tests are not reaching the server running on localhost. πŸ€”

LuchoTurtle commented 1 year ago

I've looked everywhere but none of the issues help the case. The connection to localhost seems to be not be reachable:

Again, running these tests on my computer works. And I'm running mix phx.server and running the tests against the live running server. However this seems to not be the case within github actions (for some reason, it has stopped establishing a connection on the server running within the service of the action pipeline).

nelsonic commented 1 year ago

@LuchoTurtle this is looking promising. πŸ‘Œ Assign when ready, then take a look at upgrading the project: https://github.com/dwyl/mvp/issues/322 πŸ™

LuchoTurtle commented 1 year ago

@nelsonic should be in a reviewable state. I refactored the Github Action files, splitting them into two different ones (one for PRs and another for the main branch`) to improve readability and separating concerns.

The API definition tests should now correctly execute in the wanted order and there is no need to seed data. Every resource needed to run the tests is made at Hoppscotch level.

Also refactored the review-app.md file a bit to include these new changes and fixed some typos I found along the way.

nelsonic commented 1 year ago

@LuchoTurtle do you think we could remove the line: https://github.com/dwyl/mvp/blob/f96d85429574a77a3bc00080bac49e24bc6a85e4/.github/workflows/pr.yml#L73

So that the deployment of the review_app is not dependent (Waiting for) the build ... πŸ’­ Is there an reason that these two jobs need to be run in series? Could they be run in parallel so that we aren't waiting?

If the build & test fails we just focus on that. But I feel like waiting for both jobs to run in series while we're debugging this is mega tedious.

I feel like we're burning a lot of time waiting for the CI to run to debug this ... ⏳ Reminds me of:

image

LuchoTurtle commented 1 year ago

The json file seems to already be switching properly. But for some odd reason, the first test fails

Run hopp test -e ./lib/api/fly_dev.json ./lib/api/MVP.json

Running: MVP/Tags/Create tag
 POST  https://mvp-pr-31[4](https://github.com/dwyl/mvp/actions/runs/4291630310/jobs/7477259821#step:5:5).fly.dev/api/tags ERROR
⚠ Error running request.
⚠ Error running test-script.

While the other ones all succeed. This didn't happen prior to changing the json file, which doesn't make sense. The error is not shown either and I can't reproduce this on my localhost :/

nelsonic commented 1 year ago

OK. looks like you've figured out the "hard" part of injecting the review app URL into the .json file ... πŸŽ‰ there doesn't appear to be a way of running hopp with a "verbose" flag/option ... https://docs.hoppscotch.io/cli πŸ€·β€β™‚οΈ

LuchoTurtle commented 1 year ago

That appears to have fixed it. Going to change the build-review-apps.md file before submitting for review.

LuchoTurtle commented 1 year ago

Nevermind. I don't know why Github is showing it was unsuccessful when the https://github.com/dwyl/mvp/actions/runs/4291836918 run was ...

image
LuchoTurtle commented 1 year ago

Ok, that should do it. Over to you, @nelsonic