Skip to content

Run upstream tests based on commit message#7329

Merged
jsignell merged 4 commits intodask:masterfrom
jrbourbeau:run-upstream
Mar 8, 2021
Merged

Run upstream tests based on commit message#7329
jsignell merged 4 commits intodask:masterfrom
jrbourbeau:run-upstream

Conversation

@jrbourbeau
Copy link
Member

@jrbourbeau jrbourbeau commented Mar 6, 2021

This PR enables us to run an upstream test build when a commit message contains the phrase test-upstream. We do this by adding a new check job to our existing upstream GHA workflow which utilizes xarray-contrib/ci-trigger to search the corresponding commit message for test-upstream. If it is found in the commit message then the upstream build is launched otherwise the upstream build is skipped.

The extra check job takes ~5 seconds to run, so I don't anticipate it impacting our CI backlog in a meaningful way.

Here are a couple of example CI builds which demonstrate the upstream tests being skipped or not:

@quasiben
Copy link
Member

quasiben commented Mar 6, 2021

This is really cool!

@jrbourbeau
Copy link
Member Author

jrbourbeau commented Mar 6, 2021

I was looking at the Xarray repo and noticed they were doing this -- totally agree it's a neat thing to have. We used to have this functionality back when we were on Travis CI, and I recall it being nice to have from time to time

@quasiben
Copy link
Member

quasiben commented Mar 6, 2021

Do you think this will grow to other libraries ? If so, maybe we should rename to something more specific, like test-upstream-xarray

@jrbourbeau
Copy link
Member Author

jrbourbeau commented Mar 6, 2021

Ah, I mean I was looking at Xarray's own upstream CI in their repo and saw they were able to launch their own upstream CI tests based on test-upstream being in a commit message (xref https://github.com/pydata/xarray/blob/37522e991a32ee3c0ad1a5ff8afe8e3eb1885550/.github/workflows/upstream-dev-ci.yaml#L14-L27). This PR adds similar infrastructure to what's in the Xarray repo for parsing commit messages to Dask, but the tests which are run here are still our existing upstream tests. Today our upstream tests only run on pushes to the main branch and on a cron.

@quasiben
Copy link
Member

quasiben commented Mar 6, 2021

Ah, ok! Thanks for the explanation

@jsignell
Copy link
Member

jsignell commented Mar 8, 2021

This is great! Do you know why contains(github.event.head_commit.message, 'test-upstream') didn't work as expected?

@jrbourbeau
Copy link
Member Author

jrbourbeau commented Mar 8, 2021

I don't totally remember why. Jim gave something similar a shot in #6196 and I also tried at one point but wasn't able to get things to work easily

@jsignell
Copy link
Member

jsignell commented Mar 8, 2021

Thanks for that context! There is one of these in ci-additional as well for test-hdf. Do you want to convert that one as well in this PR? Also happy to merge as is.

@jrbourbeau jrbourbeau mentioned this pull request Mar 8, 2021
Closed
@jrbourbeau
Copy link
Member Author

jrbourbeau commented Mar 8, 2021

I'm okay limiting the scope of this PR to just upstream. I've opened up #7337 to track doing something similar with HDFS tests

@jsignell jsignell merged commit b4f2e42 into dask:master Mar 8, 2021
@jrbourbeau jrbourbeau deleted the run-upstream branch March 8, 2021 18:34
dcherian added a commit to dcherian/dask that referenced this pull request Mar 18, 2021
@dcherian
Merge remote-tracking branch 'upstream/main' into sliding-window
3c327ee 
* upstream/main:
  Change default branch from master to main (dask#7198)
  Add Xarray to CI software environment (dask#7338)
  Update repartition argument name in error text (dask#7336)
  Run upstream tests based on commit message (dask#7329)
  Use pytest.register_assert_rewrite on util modules (dask#7278)
  add example on using specific chunk sizes in from_array() (dask#7330)
  Move numpy skip into test (dask#7247)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jrbourbeau @quasiben @jsignell