Skip to content
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

Update tests #789

Merged
merged 5 commits into from
Jan 31, 2024
Merged

Update tests #789

merged 5 commits into from
Jan 31, 2024

Conversation

glatterf42
Copy link
Member

@glatterf42 glatterf42 commented Jan 30, 2024

The current CI failures have multiple causes. One is pytest 8.0.0, for which this PR adjusts. The affected test was working properly, as far as I can tell, by raising a ValueError before the DeprecationWarning. In the past, since a ValueError was given as the expected result of the test, this was fine; but now, pytest wants to see the DeprecationWarning.
On other tests, one can run with pytest.raises(ValueError, match="something") to specify which ValueError is raised -- which would have made us aware of this issue long ago since Scenario.add_horizon() raises ValueErrors on multiple occasions. From pytest's docs, it doesn't look like we can pass a match= string to the xfail mark, so I'm wondering how much refactoring if would take to implement this. Another option would be to subclass ValueError and raise custom unique errors for the various fail cases, which we can then tell pytest to expect.

This PR also temporarily pins genno < 1.23 and can be used to upgrade Node.js actions as GHA recommends.

How to review

  • Read the diff and note that the CI checks all pass.

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • [ ] Add, expand, or update documentation. Just updating tests
  • [ ] Update release notes. Just updating tests

@glatterf42 glatterf42 added bug Doesn't work as advertised/unintended effects ci Continuous integration labels Jan 30, 2024
@glatterf42 glatterf42 added this to the 3.9 milestone Jan 30, 2024
@glatterf42 glatterf42 self-assigned this Jan 30, 2024
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7ffee7f) 95.2% compared to head (96041ca) 95.2%.

Additional details and impacted files
@@          Coverage Diff          @@
##            main    #789   +/-   ##
=====================================
  Coverage   95.2%   95.2%           
=====================================
  Files         46      46           
  Lines       4335    4335           
=====================================
  Hits        4130    4130           
  Misses       205     205           
Files Coverage Δ
message_ix/tests/test_core.py 100.0% <100.0%> (ø)

Copy link
Member

@khaeru khaeru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good! One change and one question, maybe a change.

pyproject.toml Outdated Show resolved Hide resolved
message_ix/tests/test_core.py Outdated Show resolved Hide resolved
@glatterf42 glatterf42 merged commit 4dae41e into iiasa:main Jan 31, 2024
24 checks passed
@glatterf42 glatterf42 deleted the fix/tests-2024-1 branch January 31, 2024 07:37
ywpratama pushed a commit to ywpratama/message_ix that referenced this pull request Jan 15, 2025
* Adjust test to code behaviour

* Bump GHA version numbers for Node.js 20

* Reify updated test

* Temporarily install genno < 1.23 in pytest CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Doesn't work as advertised/unintended effects ci Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants