Skip to content

nightly upstream test #4574

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

Closed
mathause opened this issue Nov 11, 2020 · 6 comments · Fixed by #4583
Closed

nightly upstream test #4574

mathause opened this issue Nov 11, 2020 · 6 comments · Fixed by #4583

Comments

@mathause
Copy link
Collaborator

As discussed in the call today it would be nice to remove the upstream test from our ci and setup a "nightly" job to test against the development versions of pandas etc. This should lead to less failures unrelated to specific PRs. Question: how are we going to see that the upstream build failed? Having the failure in random PRs is not clean but very visible...

This could be done via github actions. See xpublish for an example setup: https://github.com/xarray-contrib/xpublish/tree/master/.github/workflows

xref #4313

@keewis
Copy link
Collaborator

keewis commented Nov 11, 2020

I think github actions have the option to send notifications for failed CI, so those who have that enabled would have to open an issue. Ideally, however, the failed action would automatically open a issue with a generic title (something like "nightly upstream-dev CI failed") and the build log (maybe trimmed to just the error logs?). Not sure if that is possible right now, though.

When thinking about this after we ended the call, I realized we would actually be losing a bit of coverage: what happens if we remove the upstream-dev CI for PRs and a PR introduces changes incompatible with upstream-dev? We would definitely catch that using the nightly CI, but only after merging.

@max-sixty
Copy link
Collaborator

When thinking about this after we ended the call, I realized we would actually be losing a bit of coverage: what happens if we remove the upstream-dev CI for PRs and a PR introduces changes incompatible with upstream-dev? We would definitely catch that using the nightly CI, but only after merging.

That's a good point. A "allowed failure" would be even better.

Though on balance, assuming we can't do "allowed failures", I would favor more accurate test results over the immediacy of broken tests on upstream deps + new code. I'm not sure there are that many failures in the intersection of upstream deps + new code.

@jhamman
Copy link
Member

jhamman commented Nov 12, 2020

ping @andersy005 and @scottyhq who are the resident experts in the areas of github actions and automation.

In a very ideal world, a failed nightly build would open an issue (or comment on an existing one) rather than send an email. I'm sure this is not only possible, but already done elsewhere. Do you guys have thoughts on what would be doable here?

@andersy005
Copy link
Member

andersy005 commented Nov 12, 2020

Thank you for the ping, @jhamman!

In a very ideal world, a failed nightly build would open an issue (or comment on an existing one) rather than send an email.

I concur with you Joe. I think this is 💯 doable today. I'm going to work on pushing it forward today, and will submit a PR once it's ready.

@mathause
Copy link
Collaborator Author

Another small thing, the py38-flaky is allowed to fail, which means that it is always green (unless someone checks the log).

allow_failure: true

@keewis
Copy link
Collaborator

keewis commented Nov 13, 2020

yeah, that makes the CI somewhat less useful. allow_failure is used for silencing the error code of pytest:

$(environment_variables) pytest \
--junitxml=junit/test-results.xml \
--cov=xarray \
--cov-report=xml \
$(pytest_extra_flags) || [ "$ALLOW_FAILURE" = "true" ]

Not sure if that would make a difference, but maybe we should try using continueOnError instead?

Edit: we could also try to print a warning if the CI fails:

- bash: |
    $(environment_variables) pytest \
    --junitxml=junit/test-results.xml \
    --cov=xarray \
    --cov-report=xml \
    $(pytest_extra_flags) \
    || ([ "$ALLOW_FAILURE" = "true" ] && echo -e "\043#vso[task.logissue type=warning;] ignored CI failure!!")

see docs for the format of these messages. It seems they are supposed to be visible in the status report of github.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants