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

Add tests for bespin.amazon.credentials #44

Merged
merged 3 commits into from Apr 22, 2017
Merged

Conversation

atward
Copy link
Collaborator

@atward atward commented Apr 21, 2017

No description provided.

@atward
Copy link
Collaborator Author

atward commented Apr 21, 2017

Oh python2.6. You so silly.

@atward atward requested a review from delfick April 21, 2017 05:45
@@ -33,8 +33,7 @@
@mock_sts
it "verify_creds ensures account_id matches aws":
credentials = Credentials('us-west-1', 987654321, NotSpecified)
with self.assertRaises(BespinError):
credentials.verify_creds() # 987654321 != 123456789012
self.assertRaises(BespinError, credentials.verify_creds) # 987654321 != 123456789012
Copy link
Owner

@delfick delfick Apr 21, 2017

Choose a reason for hiding this comment

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

I suggest

with self.fuzzAssertRaisesError(BespinError, "message you expect", kwarg1=1, kwarg2=2):
    credentials.verify_creds()

It's defined by delfick_error (https://github.com/delfick/delfick_error/blob/master/delfick_error.py#L143)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Travis didn't like that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't see delfick_error in setup.py

Copy link
Owner

Choose a reason for hiding this comment

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

it's pulled in by delfick-app

from bespin.amazon.credentials import Credentials
from bespin.errors import BespinError
from tests.helpers import BespinCase
from input_algorithms.spec_base import NotSpecified
Copy link
Owner

Choose a reason for hiding this comment

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

I'm a bit nitpicky about imports

Usually I split into groups

<bespin imports>

<test specific imports>

<other imports>

Each block has no empty newlines and is ordered by the length of the line, longest line to shortest line.

so here, would be

from bespin.amazon.credentials import Credentials
from bespin.errors import BespinError

from tests.helpers import BespinCase

from input_algorithms.spec_base import NotSpecified
from moto import mock_sts
import os

@mock_sts
it "verify_creds ensures account_id matches aws":
credentials = Credentials('us-west-1', 987654321, NotSpecified)
with self.fuzzAssertRaisesError(BespinError, "Please use credentials for the right account", expect=self.account_id, got=amazon_account_id):
Copy link
Owner

Choose a reason for hiding this comment

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

should be fuzzy instead of fuzz

@atward atward force-pushed the moto_creds branch 2 times, most recently from 9b35c6c to 12b90c0 Compare April 21, 2017 15:16
@delfick delfick merged commit eb6b86b into delfick:master Apr 22, 2017
@delfick
Copy link
Owner

delfick commented Apr 22, 2017

So anyway, these tests were breaking in tox because of this 8b6fad9 (tox doesn't set a USER variable and so that code was breaking the response from moto)

And also, 1b79a33, environment variables from one test was changing the behaviour of another test.

@atward
Copy link
Collaborator Author

atward commented Apr 22, 2017

Would it be better/possible to mock a clone of os.environ for the particular unittests which would modify the environment?

@delfick
Copy link
Owner

delfick commented Apr 22, 2017

possibly, should be possible with mock.patch.dict https://docs.python.org/3/library/unittest.mock.html#unittest.mock.patch.dict

@atward
Copy link
Collaborator Author

atward commented Apr 26, 2017

unittest.mock is only available post python 3.3

@delfick
Copy link
Owner

delfick commented Apr 26, 2017

that's why it's a dep in setup.py https://github.com/delfick/bespin/blob/master/setup.py#L37

@atward atward deleted the moto_creds branch April 26, 2017 02:58
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.

None yet

2 participants