Skip to content

Conversation

theengineear
Copy link
Contributor

We should really refactor the test suite to prevent future
unexpected events like this.

We should really refactor the test suite to prevent future
unexpected events like this.
@theengineear
Copy link
Contributor Author

These passed locally now, but we'll wait to make sure they pass on Circle. I looked into the post params, it looks like username and key were being set in another test to ensure failure, but not being reset. This is a flaw in the test suite that should be addressed at some point.

As a side note, we only check if keys exist in the credentials file, not if they're empty. A cursory check might save a user (and us!) a lot of hassle. I was debugging this and didn't realize I'd unset the file based on the errors i was getting back from _send_to_plotly: 500 Internal Server Error.

@theengineear
Copy link
Contributor Author

yay! @etpinard , there's no problem with the library, just the test suite. I'll leave this for you to take a peek at and I'll merge tomorrow, no rush.

@etpinard
Copy link
Contributor

etpinard commented Sep 8, 2014

Thanks man 👍

theengineear added a commit that referenced this pull request Sep 17, 2014
Both username and key were being set improperly during test suite.
@theengineear theengineear merged commit 3802f96 into master Sep 17, 2014
@theengineear theengineear deleted the andrew-fix-failing-test branch September 18, 2014 08:44
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