ci: Start using github actions for CI #235

flub · 2020-06-10T09:44:20Z

Almost comprehensive replacement of CI in github actions:

TODO:

AWS and GCS keys to allow integration tests against them
Zeus docs upload
Zeus webook notifications

#skip-changelog

Swatinem

nice! do we care about other OSs for symbolicator? Might be good to use a matrix for that maybe? https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstrategymatrix

flub · 2020-06-10T10:44:03Z

The current travis config only tests on linux, so I did the same here.

jan-auer

Love it, thank you so much for getting this started!

My only issue with this is that it diverges from the Makefile, which is our canonical "source of truth" for such commands. Is it possible to invoke make * commands, particularly the ones we run in Travis at the moment?

After that, if caches turn out to be stable, I'd be more than happy to remove the Travis and AppVeyor builds.

jan-auer

Sorry, meant to request changes.

On an unrelated note, would you mind updating the PR title? The type in the beginning should match one of the types declared here: https://develop.sentry.dev/code-review/#type. Thanks!

flub · 2020-06-12T12:00:01Z

By using the actions-rs/cargo "action" thingy we get the integration which comments in the PR on the right locations for lints etc. If we want that functionality for the make target we'd have to write our own action I think. I'd rather be able to use the community maintained actions I think.

I imagined that we'd add another workflow with (some of) the makefile things in it before actually ditching travis. But my intention here wasn't actually to try and ditch travis, just try out the actions features for a while in parallel to see if it's worth investing in migrating and how we would like migration to look.

.github/workflows/rust.yml

flub · 2020-06-18T10:53:48Z

Deciding how to integrate the makefile functionality is more interesting. I'm thinking adding another workflow which invokes the makefile e2e tests is straight forward, though that already duplicates the rust tests. The overlapping linting etc is more interesting.

flub · 2020-06-18T10:55:50Z

It would be nice if there was a filter you could apply to rust's and pytest's output to turn it into action-compatible errors and lints so we could run everything from the makefile. Maybe that's too crazy.

flub · 2020-08-03T13:43:05Z

This now does everything that the travis run does, but faster. The clippy and cargofmt runs are not using the makefile so we get annotations from actions-rs. The "Rust Check" stop is entirely new and gives faster build feedback with annotations.

Bot unittest and integration tests have to do the same cargo build which should take about the same time. However because both test suites take >30s it's worth to duplicate this.

.github/workflows/ci.yml

This is only the basic rust actions, not all things done by travis currently are covered yet.

Using artifacts rather than caches since here we're explicitly including the app build itself.

We don't (yet) build a binary.

The makefile also didn't do this.

We don't actually want to upload docs for normal PRs

.github/workflows/build.yml

.github/workflows/ci.yml

Swatinem · 2020-10-02T08:27:14Z

.github/workflows/ci.yml

+        run: pip install --upgrade black flake8
+
+      - name: Run Black
+        run: black --check tests


Q: should we use this directly instead of makefile for symbolic as well?

Since we're going with explicit commands now, I would say yes. The makefile becomes an interface for the developer, and CI has it's own (equivalent or equal) commands.

.github/workflows/ci.yml

untitaker · 2020-10-02T09:06:43Z

tests/integration/conftest.py

-        pytest.skip("No AWS credentials")
+        msg = "No AWS credentials"
+        if pytestconfig.getoption("ci"):
+            pytest.fail(msg)


In theory this kind of stuff could be monitored by measuring coverage on the testsuite itself. Then you can find tests that are improperly skipped or not discovered at all for whatever reason.

In practice it's much harder to set up.

I think that could work if you have a 100% test coverage enforced. But anything less and it probably doesn't. I like one global that you can use to potentially skip things on local workstations.

I mean testing coverage of the code in tests/, not the code that is being tested. Surely all tests are running in CI?

jan-auer

craft.yml still requires the following artifacts:

symbolicator-Linux-x86_64
symbolicator-Linux-x86_64-debug.zip

Unfortunately, merging like this will break releases. The artifacts were built by the release: linux x86_64 Travis job. You can either remove the craft requirement, or add the release build back.

Nothing currently produces these binaries, this is currently broken.

flub · 2020-10-02T15:11:41Z

craft.yml still requires the following artifacts:

symbolicator-Linux-x86_64

symbolicator-Linux-x86_64-debug.zip

Unfortunately, merging like this will break releases. The artifacts were built by the release: linux x86_64 Travis job. You can either remove the craft requirement, or add the release build back.

Deleted these two from .craft.yml as I think things were broken since they were added.

Producing binaries opens up so many questions for me that I really would rather not mix that up into this PR. Apologies.

jan-auer

Deleted these two from .craft.yml as I think things were broken since they were added.

I'm not sure what makes you say that. Both the build and the craft configuration look fine to me. I remember testing this when I added it. But it's up to you, we can bring this back later.

I was mistaken that this wasn't currently done. Let's keep it.

Apparently we now have CI independently from the Makefile.

Swatinem · 2020-10-05T10:05:36Z

Note to self:

    Updating git repository `https://github.com/Keats/jsonwebtoken`
    Updating git repository `https://github.com/getsentry/symbolic`
    Updating git submodule `https://github.com/getsentry/breakpad`
    Updating git submodule `https://github.com/getsentry/linux-syscall-support`
    Updating git repository `https://github.com/Swatinem/cpp_demangle`
    Updating git repository `https://github.com/Swatinem/msvc-demangler-rust`
   Compiling symbolicator v0.2.0 (/home/runner/work/symbolicator/symbolicator)
   Compiling cpp_demangle v0.3.0 (https://github.com/Swatinem/cpp_demangle?branch=sentry-patches#2d274c48)
   Compiling symbolic-demangle v7.5.0 (https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#737969f7)
   Compiling symbolic v7.5.0 (https://github.com/getsentry/symbolic?branch=fix/demangle-fixes#737969f7)

My cache does not support git dependencies yet, but it looks like it would be a small win here

flub · 2020-10-05T10:06:53Z

Deleted these two from .craft.yml as I think things were broken since they were added.

I'm not sure what makes you say that. Both the build and the craft configuration look fine to me. I remember testing this when I added it. But it's up to you, we can bring this back later.

Ok, I read the existing travis.ymy and makefile badly again. Re-added this now. Ready to merge again I think.

I assume once this is merged with admin rights the requirement for the travis checks will disappear again?

jan-auer · 2020-10-05T13:20:54Z

Updated branch protection rules on GitHub.

flub marked this pull request as ready for review June 10, 2020 10:21

flub requested a review from a team June 10, 2020 10:21

Swatinem approved these changes Jun 10, 2020

View reviewed changes

jan-auer reviewed Jun 10, 2020

View reviewed changes

jan-auer requested changes Jun 10, 2020

View reviewed changes

flub changed the title ~~maint(ci): Start using github actions for CI~~ ci: Start using github actions for CI Jun 12, 2020

Swatinem reviewed Jun 15, 2020

View reviewed changes

.github/workflows/rust.yml Outdated Show resolved Hide resolved

Swatinem reviewed Jun 15, 2020

View reviewed changes

.github/workflows/rust.yml Outdated Show resolved Hide resolved

flub marked this pull request as draft June 22, 2020 09:21

flub marked this pull request as ready for review August 3, 2020 13:43

Swatinem reviewed Aug 4, 2020

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

.github/workflows/ci.yml Outdated Show resolved Hide resolved

Floris Bruynooghe added 14 commits August 10, 2020 15:41

maint(ci): Start using github actions for CI

7d274cc

This is only the basic rust actions, not all things done by travis currently are covered yet.

Enable workspaces, all features and locked Cargo.lock file

307e226

Add newline at end of file...

ec5cfa8

Use our clippy flags

5c2de66

Limit push build to only build for pushes to master

28ec04f

Start testing caching

b8987e1

fix typo

62aeebe

Same typo

45f5b8f

Also cache build stuff

42b372f

Try passing caches between jobs using artifacts

6bf746b

Using artifacts rather than caches since here we're explicitly including the app build itself.

typo

c7a85a4

Attempt to find where rust actually lives

f4246c7

Abandon artifacts, might as well wait for Godot

f094376

Remove unused dependency

ffbce5f

Floris Bruynooghe added 8 commits October 2, 2020 09:40

Correct syntax

50f1135

Less confusing name

82b7b14

We don't (yet) build a binary.

Use zeus directly

561bb8b

Use npx instead of yarn

b3d10bb

Do not name the upload

52dbb17

The makefile also didn't do this.

Actually zip up the docs

2eb130e

Remove travis config

9ede8d1

Remove test setup

eafab8a

We don't actually want to upload docs for normal PRs

flub requested a review from jan-auer October 2, 2020 08:18

Swatinem reviewed Oct 2, 2020

View reviewed changes

.github/workflows/build.yml Outdated Show resolved Hide resolved

Swatinem reviewed Oct 2, 2020

View reviewed changes

More suitable name

a090f8c

untitaker reviewed Oct 2, 2020

View reviewed changes

Swatinem approved these changes Oct 2, 2020

View reviewed changes

jan-auer requested changes Oct 2, 2020

View reviewed changes

Do not require binaries for releases

6045095

Nothing currently produces these binaries, this is currently broken.

flub requested a review from jan-auer October 2, 2020 15:11

jan-auer approved these changes Oct 2, 2020

View reviewed changes

Floris Bruynooghe added 5 commits October 5, 2020 10:37

Build binaries again

75c176f

I was mistaken that this wasn't currently done. Let's keep it.

Do not use makefile for docs building

49dc5ef

Apparently we now have CI independently from the Makefile.

Correct release binary location

60088f3

Quotes are hard

bc5771b

Remove temp test config

f11e867

flub requested a review from jan-auer October 5, 2020 10:07

flub merged commit fc776ca into master Oct 5, 2020

flub deleted the maint/github-actions branch October 5, 2020 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: Start using github actions for CI #235

ci: Start using github actions for CI #235

flub commented Jun 10, 2020 •

edited

Loading

Swatinem left a comment

flub commented Jun 10, 2020

jan-auer left a comment

jan-auer left a comment

flub commented Jun 12, 2020

flub commented Jun 18, 2020

flub commented Jun 18, 2020

flub commented Aug 3, 2020

Swatinem Oct 2, 2020

jan-auer Oct 2, 2020

untitaker Oct 2, 2020

flub Oct 2, 2020

untitaker Oct 2, 2020

jan-auer left a comment

flub commented Oct 2, 2020

jan-auer left a comment

Swatinem commented Oct 5, 2020

flub commented Oct 5, 2020

jan-auer commented Oct 5, 2020

ci: Start using github actions for CI #235

ci: Start using github actions for CI #235

Conversation

flub commented Jun 10, 2020 • edited Loading

Swatinem left a comment

Choose a reason for hiding this comment

flub commented Jun 10, 2020

jan-auer left a comment

Choose a reason for hiding this comment

jan-auer left a comment

Choose a reason for hiding this comment

flub commented Jun 12, 2020

flub commented Jun 18, 2020

flub commented Jun 18, 2020

flub commented Aug 3, 2020

Swatinem Oct 2, 2020

Choose a reason for hiding this comment

jan-auer Oct 2, 2020

Choose a reason for hiding this comment

untitaker Oct 2, 2020

Choose a reason for hiding this comment

flub Oct 2, 2020

Choose a reason for hiding this comment

untitaker Oct 2, 2020

Choose a reason for hiding this comment

jan-auer left a comment

Choose a reason for hiding this comment

flub commented Oct 2, 2020

jan-auer left a comment

Choose a reason for hiding this comment

Swatinem commented Oct 5, 2020

flub commented Oct 5, 2020

jan-auer commented Oct 5, 2020

flub commented Jun 10, 2020 •

edited

Loading