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

Migrate Travis CI to Github Actions #67

Merged
merged 45 commits into from
Apr 8, 2021

Conversation

louilinn
Copy link
Contributor

@louilinn louilinn commented Feb 11, 2021

Related to issue: #64

@louilinn louilinn force-pushed the travis-migration-to-github-actions branch 3 times, most recently from b750c26 to 107672f Compare February 12, 2021 09:35
@louilinn louilinn force-pushed the travis-migration-to-github-actions branch from 107672f to 2e1bf70 Compare February 12, 2021 09:56
@llunaCreixent llunaCreixent linked an issue Feb 15, 2021 that may be closed by this pull request
@louilinn louilinn force-pushed the travis-migration-to-github-actions branch 10 times, most recently from 7081726 to 4975190 Compare February 16, 2021 16:56
@louilinn louilinn force-pushed the travis-migration-to-github-actions branch from 4975190 to ccb6768 Compare February 16, 2021 16:58
@louilinn louilinn force-pushed the travis-migration-to-github-actions branch from f421131 to 9bb82de Compare February 26, 2021 11:19
@louilinn louilinn force-pushed the travis-migration-to-github-actions branch from eb8c730 to 4276afe Compare February 26, 2021 13:20
@louilinn
Copy link
Contributor Author

Actions now reach running the core tests but we get 404 from the relayer

@adzialocha found that docker-gen is having an issue with finding out the ip addresses of the created containers, so nginx-proxy doesn't know where to route the HTTP requests.

There seems to be an issue with a docker update affecting that ip-addresses are not properly added as upstream in nginx configuration
nginx-proxy/docker-gen#335
nginx-proxy/docker-gen#336

@louilinn
Copy link
Contributor Author

louilinn commented Apr 5, 2021

The above mentioned PRs are now merged / closed. However we still get the 404

FAIL test/token.test.js (7.399 s)
  ● Token › should check if safe has enough funds for token to be deployed

    RequestError: Request failed @ http://relay.circles.local/api/v3/safes/predict/ with error 404

      70 |       } else {
      71 |         if (response.status >= 400) {
    > 72 |           throw new RequestError(url, response.body, response.status);
         |                 ^
      73 |         }
      74 | 
      75 |         return response.body;

      at src/utils.js:72:17

@adzialocha
Copy link
Collaborator

The above mentioned PRs are now merged / closed. However we still get the 404

FAIL test/token.test.js (7.399 s)
  ● Token › should check if safe has enough funds for token to be deployed

    RequestError: Request failed @ http://relay.circles.local/api/v3/safes/predict/ with error 404

      70 |       } else {
      71 |         if (response.status >= 400) {
    > 72 |           throw new RequestError(url, response.body, response.status);
         |                 ^
      73 |         }
      74 | 
      75 |         return response.body;

      at src/utils.js:72:17

Was there a new version released / do we use it already?

@louilinn
Copy link
Contributor Author

louilinn commented Apr 6, 2021

Should we update the version here?

Also the issue seems to remain when using a custom hostname (which i think we do here) according to comments in the PRs mentioned.

@adzialocha
Copy link
Collaborator

Should we update the version here?

Yes, super!

@adzialocha adzialocha marked this pull request as ready for review April 7, 2021 16:24
@adzialocha adzialocha self-requested a review as a code owner April 7, 2021 16:24
@adzialocha adzialocha requested a review from llunaCreixent April 7, 2021 16:24
@adzialocha
Copy link
Collaborator

adzialocha commented Apr 7, 2021

I think this is ready @louilinn and @llunaCreixent ! 🥳

I've updated docker-gen to 0.7.5 in circles-docker and removed tmate. Also all circles tests are passing now (locally): #63

To test this finally we need to merge the fixed tests with this CI update, so I suggest to bring both PRs into main and troubleshoot from there (when there is something missing).

llunaCreixent
llunaCreixent previously approved these changes Apr 8, 2021
Copy link
Member

@llunaCreixent llunaCreixent left a comment

Choose a reason for hiding this comment

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

Let's merge! great job!

.github/workflows/tests.yml Outdated Show resolved Hide resolved
@llunaCreixent
Copy link
Member

Tests failing. See:
https://github.com/CirclesUBI/circles-core/runs/2294779014?check_suite_focus=true

Copy link
Collaborator

@adzialocha adzialocha left a comment

Choose a reason for hiding this comment

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

🥳 🥳 🥳 🥳 🥳

Woho! I changed the tests to --runInBand and it works, I really think we have a race-condition bug in the relayer (started a ticket here but will add more info soon: CirclesUBI/safe-relay-service#44). For now we keep that flag and move back to concurrently running tests when the bug was fixed (created an issue here: #75).

@louilinn if you could remove that PostgreSQL part (see feedback of @llunaCreixent), I'm happy to merge it (I have the admin rights to merge it even though the Travis CI is blocking) 🌴 🐧

@louilinn
Copy link
Contributor Author

louilinn commented Apr 8, 2021

The PostgreSQL part is already gone :) I removed the travis file. I think we should squash all these commits when merging haha ^^

@louilinn louilinn requested a review from adzialocha April 8, 2021 14:11
@adzialocha adzialocha merged commit 7c72207 into main Apr 8, 2021
@adzialocha adzialocha deleted the travis-migration-to-github-actions branch April 8, 2021 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move CI from Travis to GitHub action
3 participants