Skip to content

Conversation

@valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Nov 28, 2022

Description

The ESGF node at CEDA has been down for a few good hours now, and our tests are failing and rightly so! But this is the last thing we'd want to see when, say, we make a release and the package fails to build due to an annoying, and independent of us issue. I marked those tests as xfailed only when the exception occurs; most of the time it doesn't so we are still testing correctly - this id done like this:

  • if test fails with requests HTTP error then it's marked as xfailed
  • if test fails with any other exception then it's marked as red failed
  • if test passes (normal behaviour) then it's marked as xpassed

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@codecov
Copy link

codecov bot commented Nov 28, 2022

Codecov Report

Merging #1830 (2f2bbe3) into main (54385ac) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1830   +/-   ##
=======================================
  Coverage   91.52%   91.52%           
=======================================
  Files         203      203           
  Lines       10920    10920           
=======================================
  Hits         9995     9995           
  Misses        925      925           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@bouweandela
Copy link
Member

Good idea to fix this, but shall we fix the code instead of the tests? I was doing a tutorial on the tool today with some people and we had to modify everyone's ~/.esmvaltool/esgf-pyclient.yml file so it uses a different order of the index nodes.

@bouweandela
Copy link
Member

Created an issue to report the problem: #1833

@valeriupredoi
Copy link
Contributor Author

Good idea to fix this, but shall we fix the code instead of the tests? I was doing a tutorial on the tool today with some people and we had to modify everyone's ~/.esmvaltool/esgf-pyclient.yml file so it uses a different order of the index nodes.

Indeed, a code fix is better, but I think we should have a failsafe in place in testing too. Xpassed is better than failed 😁

@valeriupredoi
Copy link
Contributor Author

Created an issue to report the problem: #1833

Cool! I also reported the index node issue to CEDA 👍

@bouweandela
Copy link
Member

bouweandela commented Nov 28, 2022

Just created a pull request to fix the issue: #1833

@bouweandela
Copy link
Member

bouweandela commented Nov 28, 2022

we should have a failsafe in place in testing too. Xpassed is better than failed grin

Every user of ESMValCore with the default ESGF configuration is affected by this problem, so to me it seems really good that our unit test picked this up.

@valeriupredoi
Copy link
Contributor Author

we should have a failsafe in place in testing too. Xpassed is better than failed grin

Every user of ESMValCore with the default ESGF configuration is affected by this problem, so to me it seems really good that our unit test picked this up.

you are correct, man, didn't realize that until today 😁 Am closing this since you fixed it proper in #1834

@valeriupredoi valeriupredoi deleted the xfail_esgf_searh_test branch November 29, 2022 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants