search

google / emboss

Emboss is a tool for generating code that reads and writes binary data structures.
Apache License 2.0
70 stars 21 forks source link

Add Github action to run Bazel tests for every PR #159

Closed jasongraffius closed 1 month ago

jasongraffius commented 1 month ago

This will ensure that all pull requests must pass all Bazel tests in the repository. Merging the pull request will be blocked until the tests are passing, and will re-run each time the pull request is synchronized or updated.

jasongraffius commented 1 month ago

For an example of what this looks like, see the check for "Run Bazel tests / run-bazel-tests (pull_request)" under "Show all checks" below. The run is configured to use Bazel's caching so subsequent runs of small PRs should be fairly quick.

jasongraffius commented 1 month ago

This is a big improvement!

I'm fine landing as-is. I can imagine wanting to expand things as follow ups. A few off the top of my head:

  • auto-formatting

Good point! I didn't think about auto-formatting. Agreed, I think this would be an improvement as well, but probably want to resolve #76 first so that there's a standard formatting.

  • pre-submit hook

Haha, you've read my mind with this one, I was putting one together but had other obligations over the weekend, I'll probably have something ready for this soon.

  • type checking (mypy or something like that)

Same thing! Read my mind, I was looking into this too! My typecheck failed locally when I tried this, but I was looking into any configuration I need to do first. If we get a passing typecheck it will be straightforward to add this.

There's also semi-related #136 where our minimum python version, 3.8, fails to run. Since this action isn't failing I'm assuming whatever version is in stock ubuntu-latest is fine, but we might consider either specifying a version to install or landing something like #137 where we configure bazel to use a hermetic python install.

It also might make sense to expand to run tests against all supported python versions (and maybe a few c++ compilers).

We should just file some issues and follow-up there, but I thought it'd be helpful to think about while you're doing this.

Well now that's three in a row, haha. I was considering adding an action that runs unit tests in both an "earliest-supported" and a "latest-tested" python versions, to guard both against breaking support for old versions and to catch any issues from upcoming versions (though the latter should be much, much more rare considering the intended API guarantees of python 3). That said, I think we should also set the hermetic python version for Bazel since we want it to always use a supported version.

jasongraffius commented 1 month ago

Added issues #160, #161, and #162 to cover the cases discussed here.