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

Switch from pyvows to nosetest #139

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

kkopachev
Copy link
Collaborator

Figured we should switch to nosetests from pyvows.

  • thumbor itself switched already
  • I have doubts vows were actually testing anything
  • I think that would help with Python3 migration

storage.put(IMAGE_BYTES, callback=callback)

def should_be_in_catalog(self, topic):
expect(topic.args[0]).to_equal([sha1('my-image.jpg'.encode('utf8')).hexdigest(), 'my-image.jpg'].join('/'))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This line has error. list does not have method join. So this test should be always failing. However it was doing in CI. There were few other similar tests which test non-existing return of a function, etc.
That all makes me think that vows were not testing anything really, just exceptions thrown in topic stage would trigger test failure, but should_ methods never worker, it seems.

Copy link

Choose a reason for hiding this comment

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

Yeah there were a few issues indeed. Thanks a lot for switching the lib :)

@Bladrak
Copy link

Bladrak commented Jan 17, 2020

Wow great work, thanks a lot! Looks good so far, there's only one test failing in the CI, would you mind looking into it? https://circleci.com/gh/thumbor-community/aws/248?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@kkopachev
Copy link
Collaborator Author

@Bladrak test fixed, should be good to go.
I've changed signatures (added @return_future) of put_crypto and put_detector_data, but I didn't find any calls to them, so I think it's safe anyway.

@Bladrak
Copy link

Bladrak commented Jan 23, 2020 via email

@Bladrak
Copy link

Bladrak commented Feb 3, 2020

@kkopachev Merged, however the build seems to be failing, would you mind checking it?

@Bladrak Bladrak mentioned this pull request Feb 3, 2020
@kkopachev kkopachev deleted the nosetests branch February 3, 2020 18:32
@kkopachev
Copy link
Collaborator Author

Looks like it fails to checkout from git. Maybe you rotated your ssh key?

@Bladrak
Copy link

Bladrak commented Feb 4, 2020

Hmm no but looks like it's an issue with the docker image, checking it. I'll let you know

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