Skip to content
This repository has been archived by the owner on Sep 19, 2023. It is now read-only.

Replace TravisCI with GitHub Actions #45

Merged
merged 2 commits into from
Jun 7, 2021

Conversation

anoadragon453
Copy link
Member

@anoadragon453 anoadragon453 commented Jun 1, 2021

Replaces TravisCI (which has officially been shut down) with Github Actions, which now runs the unit tests.

You can see the pipeline passing over on my repo: https://github.com/anoadragon453/matrix-content-scanner/runs/2718947043

Signed-off-by: Andrew Morgan <andrewm@element.io>

@anoadragon453 anoadragon453 requested a review from a team June 1, 2021 13:20
Copy link

@callahad callahad left a comment

Choose a reason for hiding this comment

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

One nit, otherwise looks fine as a port of the Travis config.

More broadly, it might make sense to get out of Docker for running tests (speed, caching, etc.), but that's out of scope for this.

@@ -0,0 +1,13 @@
name: Unit tests
on: [push]
Copy link

Choose a reason for hiding this comment

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

You probably want this to trigger on all pull requests, but only on pushes to the main branch. Otherwise a push to an open PR will trigger the workflow to run twice

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would only run twice if I'd specified [push, pull_request] here? I tried making a PR and only had one test run occur: anoadragon453#1

Copy link

@callahad callahad Jun 2, 2021

Choose a reason for hiding this comment

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

Ah, what I mean is that you probably want it to run on pull_request as well, since otherwise I don't think PRs from external forks wouldn't trigger the workflow? And if you're running on pull_request, you'll need to limit the branches for the push rule.

But I could be wrong. Should we merge it and see?

Copy link
Member Author

Choose a reason for hiding this comment

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

Aha, I didn't think about how pull wouldn't cover external repositories. I actually cribbed this behaviour from Sygnal: https://github.com/matrix-org/sygnal/blob/main/.github/workflows/pipeline.yml#L2

And it indeed looks like tests aren't being run on contributor PRs: matrix-org/sygnal#227 I'll PR a fix to Sygnal as well as update this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think ideally I'd want to have tests run on a push to any branch on the repository, even if it's not part of a pull request. However, I'm not sure how to achieve that automatically without also triggering two jobs to run when pushing to a PR branch.

It also doesn't look like it's possible to manually run a workflow on an arbitrary branch alas.

Copy link

@callahad callahad left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

@anoadragon453 anoadragon453 merged commit 030831e into matrix-org:main Jun 7, 2021
@anoadragon453 anoadragon453 deleted the anoa/github_actions branch June 7, 2021 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants