Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix deployment tests, skip tests requiring optional dependencies #818

Merged
merged 13 commits into from
Feb 16, 2021

Conversation

mattwthompson
Copy link
Member

This PR fixes a small typo in the deployment tests (that would have been quite difficult to see ahead of time in #803). This is contained to just this workflow; the release is not broken. When comparing git tags, the runner steps up one directory before running git ls-remote to find the tag of the cloned repo, which is then compared to the version found in the Python package. This check works fine and passes. But when going back into the repo, it needs to be openff-toolkit, not openff/toolkit. So this line is an exception to the general pattern of which path needs to be stepped into.

I'll see if I can run the workflow from this branch to be sure it works.

@mattwthompson
Copy link
Member Author

This link is probably not persistent, but the failing check is fixed and the tests are passing so far: https://github.com/openforcefield/openff-toolkit/runs/1729531265?check_suite_focus=true

@codecov
Copy link

codecov bot commented Jan 19, 2021

Codecov Report

Merging #818 (d99459e) into master (6e11ed6) will decrease coverage by 0.14%.
The diff coverage is n/a.

@mattwthompson
Copy link
Member Author

mattwthompson commented Jan 25, 2021

To clarify, this PR does two things

  1. Fixes the openff-toolkit path while the runner is moving around in the directories
  2. Marks some tests as xfail

# 1 will work before a release, but # 2 will not work until the next release because this action checks out the most recent commit. But at least the tests will run, and only fail for a small set of known "failures."

@mattwthompson mattwthompson requested a review from j-wags January 26, 2021 17:14
@mattwthompson mattwthompson changed the title Fix a path in deployment tests Fix deployment tests, skip tests requiring optional dependencies Jan 27, 2021
Copy link
Member

@j-wags j-wags left a comment

Choose a reason for hiding this comment

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

Looks good, and the test_molecule changes are really nice too. For the tests where OE and RDKit gave insignificantly different results (eg different precisions), I've changed them to be separate tests. Thanks, @mattwthompson!

@j-wags j-wags merged commit 167b874 into master Feb 16, 2021
@j-wags j-wags deleted the fix-deployment-tests branch February 16, 2021 01:39
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants