Skip to content
This repository has been archived by the owner on Aug 15, 2022. It is now read-only.

Add tests and bugfix for b'' messages appearing in python3. #46

Merged
merged 2 commits into from
Jun 5, 2016

Conversation

jammons
Copy link
Contributor

@jammons jammons commented May 28, 2016

I added unicode and non-unicode message testing for the output() method
on RTMBot. I'm concerned about why the encode('ascii', 'ignore') was
there in the first place, but not having it seems to do the right
thing...

Fixes: #45

I added unicode and non-unicode message testing for the output() method
on RTMBot. I'm concerned about why the encode('ascii', 'ignore') was
there in the first place, but not having it seems to do the right
thing...
@coveralls
Copy link

coveralls commented May 28, 2016

Coverage Status

Coverage increased (+5.8%) to 34.194% when pulling 2b9e9b7 on jammons:master into 006e28f on slackhq:master.

@jammons
Copy link
Contributor Author

jammons commented May 28, 2016

This fixes the same issue as in https://github.com/slackhq/python-rtmbot/pull/44/files and if the ascii encode is necessary, doing the decode is another solution to this problem. (I'd like to try to move to using utf8 in python 2.7, so maybe encode('utf8').decode())?

rtmbot = init_rtmbot()

# Mock the slack_client object
slackclient_mock = Mock()
Copy link
Contributor

Choose a reason for hiding this comment

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

You want to create_autospec here.

from slackclient import SlackClient

slackclient_mock = mock.create_autospec(SlackClient, spec_set=True)

For the above and all other mocks. This is going to save you from making a typo somewhere and actually ensure that you're writing tests against real methods. https://docs.python.org/3/library/unittest.mock.html#auto-speccing has a good write up on more details- happy to discuss further, since it took me a long while to grok!

@coveralls
Copy link

coveralls commented Jun 5, 2016

Coverage Status

Coverage increased (+6.2%) to 34.615% when pulling c12244a on jammons:master into 006e28f on slackhq:master.

Strings should now be passed to the plugins as unicode values in py2.7
so we have to make sure that we don't convert those into ascii by using
python str instead of u'' strings.
@coveralls
Copy link

coveralls commented Jun 5, 2016

Coverage Status

Coverage increased (+5.8%) to 34.194% when pulling c81bdfe on jammons:master into 006e28f on slackhq:master.

@jammons jammons merged commit 1a932f9 into slackapi:master Jun 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants