-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
CI: adding remote-data test job, remove duplicate #358
Conversation
425d2bd
to
bed3ffb
Compare
bed3ffb
to
62762d9
Compare
Codecov Report
@@ Coverage Diff @@
## main #358 +/- ##
=======================================
Coverage 78.36% 78.36%
=======================================
Files 46 46
Lines 5506 5506
=======================================
Hits 4315 4315
Misses 1191 1191
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
0ce01a1
to
648924b
Compare
While you are at it, are you interested in using the OpenAstronomy template? @Cadair has a PR out for |
Failure looks real?
|
On Mon, Sep 19, 2022 at 05:36:53AM -0700, P. L. Lim wrote:
Failure looks real?
```
E AssertionError: assert 'This data co...bscore table.' == 'This data co...bscore table.'
E - This data collection is queriable in GAVO Data Center's obscore table.
E ? ^
E + This data collection is queryable in GAVO Data Center's obscore table.
E ?
```
Oh dang, yes. Or rather, no, this is not a pyVO failure. I've let
codespell run on DaCHS, and that's one of the (many) misspellings it
found.
Can you include a fix for that with your PR? That'd be most
appreciated.
|
not this time, but thanks for the heads up. |
Hmm, new error now: |
Known one, I'm pretty sure that's due to the offline CADC mentioned above. I'll restart CI tomorrow when they are back online, and expect an all-green CI (bar the narrative docs that may have some genuine failures that I'll either fix or skip for now in order to not hold up the release). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just looking at the tox and workflow file, things look good. Good luck with the CADC stuff!
thank you Pey Lian! |
CADC is slowly coming back after a scheduled maintenance this weekend (big power upgrade to the data centre). I'm fine with removing the matrix and the allocation of Python versions to targets is logical but in order to preserve that in the |
I like to think spelling out this explicitly is easier to see right away what's going on, after all I don't think we would save much lines of config with introducing the complexity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bsipocz , this looks good to me as well. Do you want to wait for it to go green before we merge?
I might need to follow up myself on the CADC failure. it could be broken for other reasons but went unnoticed with the remote-data tests not part of the CI. |
Yes, keep this until all is green, either as fixed of intentionally skipped. |
@andamian - I see this (or one very much similar) error with cadc in astroquery. Do you want me to open a separate issue somewhere about it? |
If CADC will take a while to resolve, maybe mark it as xfail and move on? If you have xpass strict, it will fail when the service is back up and the test no longer xfail. |
yes, the question now is whether this is an issue with pyvo, because then we should fix it today for the release. If it's fully upstream then I agree, an xfail is a good workaround. |
This one is the example from astroquery running into this, too:
|
62a9f1a
to
217c041
Compare
eb09715
to
a22ee0a
Compare
This got the approval and we discussed the skipping/xfailing of the remaining issues, so I go ahead and merge this now. |
This PR reshuffles the CI a bit in order to:
--remote-data=any
to one of the jobs.As a result now we test all OSes with all the dependencies while keeping one linux run with the mandatory dependencies only. We also have one remote-data test and one for the oldest dependency versions, and one for the dev versions. I feel with the reshuffle we cover more of our grounds with one less job.
note: I expect the remote-data job to fail until CADC is back online. If it keeps failing after that then we have to deal with a genuine issues.
I would love it if @pllim could have a look at this to whether I've missed a piece of logic somewhere.