Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PR] Fix CI API tests failing #314

Merged
merged 29 commits into from
Feb 28, 2023
Merged

[PR] Fix CI API tests failing #314

merged 29 commits into from
Feb 28, 2023

Conversation

LuchoTurtle
Copy link
Member

@LuchoTurtle LuchoTurtle commented Feb 21, 2023

closes #297
closes #315

@LuchoTurtle LuchoTurtle added in-progress An issue or pull request that is being worked on by the assigned person chore a tedious but necessary task often paying technical debt labels Feb 21, 2023
@LuchoTurtle LuchoTurtle self-assigned this Feb 21, 2023
@LuchoTurtle LuchoTurtle added the bug Suspected or confirmed bug (defect) in the code label Feb 21, 2023
@codecov
Copy link

codecov bot commented Feb 21, 2023

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           

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

@LuchoTurtle
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member

@LuchoTurtle this is looking promising. 👌
Assign when ready, then take a look at upgrading the project: #322 🙏

@LuchoTurtle LuchoTurtle added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Feb 27, 2023
@LuchoTurtle LuchoTurtle removed their assignment Feb 27, 2023
@@ -0,0 +1,12 @@
{
"host": "https://mvp-pr-314.fly.dev",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this hard-coding of the host mean that this will only work while the 314 PR is open? 💭
What happens to the next PR that doesn't have this ID?

Copy link
Member Author

@LuchoTurtle LuchoTurtle Feb 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nelsonic dang it, I thought I had gotten everything covered. I knew one last step was missing. You are right, this was a temporary fix just to see if the runs were being correctly executed on my local machine back-to-back.

Please assign the PR back to me, I will address this. Sorry about this, completely forgot this last step.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing. re-assigned to you. Please review your own PRs before assigning them to ensure you have checked everything. 🙏

@nelsonic nelsonic added in-progress An issue or pull request that is being worked on by the assigned person and removed in-review Issue or pull request that is currently being reviewed by the assigned person labels Feb 27, 2023
@nelsonic nelsonic assigned LuchoTurtle and unassigned nelsonic Feb 27, 2023
@nelsonic
Copy link
Member

@LuchoTurtle do you think we could remove the line:

needs: [build]

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
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

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

@LuchoTurtle
Copy link
Member Author

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
Copy link
Member Author

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

@LuchoTurtle LuchoTurtle added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Feb 28, 2023
@LuchoTurtle LuchoTurtle assigned nelsonic and unassigned LuchoTurtle Feb 28, 2023
Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LuchoTurtle looks good. 👌
Really hope this is more robust now. 🤞🏼
Thanks. ☀️

@nelsonic nelsonic merged commit a30aa78 into main Feb 28, 2023
@nelsonic nelsonic deleted the fix-CI-#297 branch February 28, 2023 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review An issue or pull request that needs to be reviewed bug Suspected or confirmed bug (defect) in the code chore a tedious but necessary task often paying technical debt
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Chore: Run API Tests against Review APP Chore: API Tests Failing on main branch
2 participants