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

Updating Pytest #120

Merged
merged 18 commits into from
May 30, 2019
Merged

Updating Pytest #120

merged 18 commits into from
May 30, 2019

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented May 30, 2019

This is a continuation of #117 to get the last Python 2 tests working.

Flamefire and others added 13 commits May 29, 2019 11:40
Adjust cache keys
Assert version after (potential) installation
Fixes #118
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
@vsoch
Copy link
Member Author

vsoch commented May 30, 2019

okay, so one of the test errors is because the semantic version string isn't being correctly parsed for the 3.2.1 install. The Singularity version, installed from the release, is:

singularity version 3.2.1-1

Which technically should return true when we test again 3.2.1, but it doesn't.

from semver import VersionInfo                                                                         

cli.version_info() >= VersionInfo(3, 2, 1)                                                             
False

We can update to 3.2.0 (which I think should work)

cli.version_info() >= VersionInfo(3, 2, 0)                                                             
True

vsoch added 4 commits May 30, 2019 11:48
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
…hich sometimes is returning unicode instead of str, thus doesnt parse and incorrectly returns None

Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
@vsoch
Copy link
Member Author

vsoch commented May 30, 2019

Cool, we just got all green! Since this updates test for pytest, and a few other PRs are depending on this, I'm going to merge. I didn't update to use Python decorators for pytest, and we I've created an issue so we don't forget to do this. To summarize the issues that were getting in the way above:

  • the Circle Orb was not honoring the singularity-verison envar, so no matter what, we were getting 3.0.1. I've fixed this and released the fix as 1.0.4.
  • Even with the correct version, the version from Singularity 3.2.1 installed from the release (3.2.1-1) was not triggering the if statement. I changed it to be >= 3.2.0, which should more cleanly separate the two.
  • Finally, the singularity oci state was strangely returning as None, and this was because the function to pipe the command was getting a unicode result, and not a string.

Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
@vsoch
Copy link
Member Author

vsoch commented May 30, 2019

There are enough changes (and an added dependency) to warrant a version bump. I'll bump, merge, release, and then get back to the other issues discussed.

@vsoch vsoch merged commit 0085f64 into master May 30, 2019
@vsoch vsoch deleted the pytest branch May 30, 2019 17:00
@Flamefire
Copy link
Contributor

How come there are so many changes here after #117 was merged?

@@ -76,6 +76,7 @@ disable=attribute-defined-outside-init,
no-member,
protected-access,
R,
unidiomatic-typecheck,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? Wouldn't isinstance(foo, str) work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don’t know why, but it didn’t. The type was Unicode in Circle and it was best caught by checking for string. If I added Unicode to the isinstance list it would skip over and return None for the state. But this works.

print('...Case 4. Check status of existing bundle.')
state = self.cli.oci.state(self.name, sudo=True)
self.assertEqual(state['status'], 'created')
if self.cli.version_info() >= VersionInfo(3, 2, 0):
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this isn't correct. Can you setup a test job with 3.2.0? I think the change was to 3.2.1 so this may fail

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure I’ll add to the other PR. If that doesn’t work do you know how to account for the last 1 in 3.3.1-1? The original check you had for >= 3.2.1 returned False.

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 this pull request may close these issues.

2 participants