Add integration tests and improve the the stability of existing unit tests #653
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seratch
commented
Apr 20, 2020
| @@ -111,7 +145,7 @@ def run(self): | |||
| include_package_data=True, | |||
| license="MIT", | |||
| classifiers=[ | |||
| "Development Status :: 2 - Pre-Alpha", | |||
| "Development Status :: 5 - Production/Stable", | |||
There was a problem hiding this comment.
This has been incorrect since version 2.0.0 release. Allow me to include this fix.
| @@ -124,12 +158,16 @@ def run(self): | |||
| ], | |||
| keywords="slack slack-web slack-rtm chat chatbots bots chatops", | |||
| packages=find_packages( | |||
| exclude=["docs", "docs-src", "tests", "tests.*", "tutorial"] | |||
| exclude=["docs", "docs-src", "integration_tests", "tests", "tests.*", "tutorial"] | |||
There was a problem hiding this comment.
the new director won't be included in the package.
`python3 setup.py sdist bdist_wheel --universal` outputs
$ python3 setup.py sdist bdist_wheel --universal
running sdist
running egg_info
writing slackclient.egg-info/PKG-INFO
writing dependency_links to slackclient.egg-info/dependency_links.txt
writing requirements to slackclient.egg-info/requires.txt
writing top-level names to slackclient.egg-info/top_level.txt
reading manifest file 'slackclient.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'slackclient.egg-info/SOURCES.txt'
running check
creating slackclient-2.6.0
creating slackclient-2.6.0/slack
creating slackclient-2.6.0/slack/rtm
creating slackclient-2.6.0/slack/web
creating slackclient-2.6.0/slack/web/classes
creating slackclient-2.6.0/slackclient.egg-info
copying files to slackclient-2.6.0...
copying LICENSE -> slackclient-2.6.0
copying MANIFEST.in -> slackclient-2.6.0
copying README.md -> slackclient-2.6.0
copying setup.cfg -> slackclient-2.6.0
copying setup.py -> slackclient-2.6.0
copying slack/__init__.py -> slackclient-2.6.0/slack
copying slack/errors.py -> slackclient-2.6.0/slack
copying slack/py.typed -> slackclient-2.6.0/slack
copying slack/version.py -> slackclient-2.6.0/slack
copying slack/rtm/__init__.py -> slackclient-2.6.0/slack/rtm
copying slack/rtm/client.py -> slackclient-2.6.0/slack/rtm
copying slack/web/__init__.py -> slackclient-2.6.0/slack/web
copying slack/web/base_client.py -> slackclient-2.6.0/slack/web
copying slack/web/client.py -> slackclient-2.6.0/slack/web
copying slack/web/slack_response.py -> slackclient-2.6.0/slack/web
copying slack/web/classes/__init__.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/actions.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/attachments.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/blocks.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/dialog_elements.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/dialogs.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/elements.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/interactions.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/messages.py -> slackclient-2.6.0/slack/web/classes
copying slack/web/classes/objects.py -> slackclient-2.6.0/slack/web/classes
copying slackclient.egg-info/PKG-INFO -> slackclient-2.6.0/slackclient.egg-info
copying slackclient.egg-info/SOURCES.txt -> slackclient-2.6.0/slackclient.egg-info
copying slackclient.egg-info/dependency_links.txt -> slackclient-2.6.0/slackclient.egg-info
copying slackclient.egg-info/requires.txt -> slackclient-2.6.0/slackclient.egg-info
copying slackclient.egg-info/top_level.txt -> slackclient-2.6.0/slackclient.egg-info
Writing slackclient-2.6.0/setup.cfg
Creating tar archive
removing 'slackclient-2.6.0' (and everything under it)
running bdist_wheel
running build
running build_py
installing to build/bdist.macosx-10.15-x86_64/wheel
running install
running install_lib
creating build/bdist.macosx-10.15-x86_64/wheel
creating build/bdist.macosx-10.15-x86_64/wheel/slack
copying build/lib/slack/version.py -> build/bdist.macosx-10.15-x86_64/wheel/slack
creating build/bdist.macosx-10.15-x86_64/wheel/slack/web
copying build/lib/slack/web/slack_response.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web
creating build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/elements.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/actions.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/__init__.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/dialog_elements.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/attachments.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/messages.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/interactions.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/dialogs.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/blocks.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/classes/objects.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web/classes
copying build/lib/slack/web/base_client.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web
copying build/lib/slack/web/client.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web
copying build/lib/slack/web/__init__.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/web
copying build/lib/slack/__init__.py -> build/bdist.macosx-10.15-x86_64/wheel/slack
creating build/bdist.macosx-10.15-x86_64/wheel/slack/rtm
copying build/lib/slack/rtm/client.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/rtm
copying build/lib/slack/rtm/__init__.py -> build/bdist.macosx-10.15-x86_64/wheel/slack/rtm
copying build/lib/slack/py.typed -> build/bdist.macosx-10.15-x86_64/wheel/slack
copying build/lib/slack/errors.py -> build/bdist.macosx-10.15-x86_64/wheel/slack
running install_egg_info
Copying slackclient.egg-info to build/bdist.macosx-10.15-x86_64/wheel/slackclient-2.6.0-py3.8.egg-info
running install_scripts
adding license file "LICENSE" (matched pattern "LICEN[CS]E*")
creating build/bdist.macosx-10.15-x86_64/wheel/slackclient-2.6.0.dist-info/WHEEL
creating 'dist/slackclient-2.6.0-py2.py3-none-any.whl' and adding 'build/bdist.macosx-10.15-x86_64/wheel' to it
adding 'slack/__init__.py'
adding 'slack/errors.py'
adding 'slack/py.typed'
adding 'slack/version.py'
adding 'slack/rtm/__init__.py'
adding 'slack/rtm/client.py'
adding 'slack/web/__init__.py'
adding 'slack/web/base_client.py'
adding 'slack/web/client.py'
adding 'slack/web/slack_response.py'
adding 'slack/web/classes/__init__.py'
adding 'slack/web/classes/actions.py'
adding 'slack/web/classes/attachments.py'
adding 'slack/web/classes/blocks.py'
adding 'slack/web/classes/dialog_elements.py'
adding 'slack/web/classes/dialogs.py'
adding 'slack/web/classes/elements.py'
adding 'slack/web/classes/interactions.py'
adding 'slack/web/classes/messages.py'
adding 'slack/web/classes/objects.py'
adding 'slackclient-2.6.0.dist-info/LICENSE'
adding 'slackclient-2.6.0.dist-info/METADATA'
adding 'slackclient-2.6.0.dist-info/WHEEL'
adding 'slackclient-2.6.0.dist-info/top_level.txt'
adding 'slackclient-2.6.0.dist-info/RECORD'
removing build/bdist.macosx-10.15-x86_64/wheel
| @@ -12,7 +12,7 @@ | |||
|
|
|||
| class TestRTMClient(unittest.TestCase): | |||
| def setUp(self): | |||
| self.client = slack.RTMClient(token="xoxp-1234", auto_reconnect=False) | |||
| self.client = slack.RTMClient(token="xoxp-1234", auto_reconnect=False, loop=asyncio.get_event_loop()) | |||
There was a problem hiding this comment.
In any case, this will be necessary when we add more tests.
| num = 1000 | ||
| clients = [] | ||
| for i in range(num): | ||
| clients.append(WebClient(token="xoxb-test", run_async=False)) |
There was a problem hiding this comment.
WebClient currently never generates a large number of event loops internally.
While working on a possible solution to address concurrency issues with run_sync=False clients, I was assessing the possibility to have asyncio.new_event_loop() and hold an independent loop inside. I found it may work in small scripts but it causes the file descriptor issue as shown in the above case (test_the_cost_of_event_loop_creation) if the app creates 100+ instances.
So, these tests are preventive measures of making such wrong decisions, just in case.
| @@ -39,6 +39,8 @@ def setUp(self): | |||
|
|
|||
| def tearDown(self): | |||
| self.loop.run_until_complete(self.site.stop()) | |||
| if not self.loop.is_closed(): | |||
| @@ -74,7 +74,7 @@ def stop_on_open(**payload): | |||
| self.assertEqual(rtm_client._connection_attempts, 2) | |||
| rtm_client.stop() | |||
|
|
|||
| client = slack.RTMClient(token="xoxa-1234", auto_reconnect=True) | |||
| client = slack.RTMClient(token="xoxa-1234", auto_reconnect=True, loop=self.loop) | |||
| if current_cpu_usage > TestRTMClient.cpu_usage: | ||
| TestRTMClient.cpu_usage = current_cpu_usage | ||
|
|
||
| TestRTMClient.cpu_monitor = threading.Thread(target=run_cpu_monitor, args=[self]) |
There was a problem hiding this comment.
Although the CPU usage of running this thread in the same process is ignorable, it may be also fine to have another process instead.
seratch
commented
Apr 20, 2020
| self.assertEqual(self.sent_text, text) | ||
|
|
||
| # TODO: Fix this issue | ||
| @pytest.mark.skip("This needs to be fixed - https://github.com/slackapi/python-slackclient/issues/569") |
There was a problem hiding this comment.
I will remove this mark in the PR addressing #569
| # TODO: Fix this issue | ||
| @pytest.mark.skip | ||
| @async_test | ||
| async def test_pagination_with_iterator_async(self): |
There was a problem hiding this comment.
No one has reported this issue but run_async=True mode doesn't allow pagination using next_cursor iterators. I'm not sure about the best way to resolve this but at least, we need to change the internals not to call run_until_complete.
|
TODO:
|
|
TODO:
|
Codecov Report
@@ Coverage Diff @@
## master #653 +/- ##
==========================================
+ Coverage 86.67% 87.14% +0.47%
==========================================
Files 15 15
Lines 1749 1821 +72
Branches 99 99
==========================================
+ Hits 1516 1587 +71
- Misses 201 202 +1
Partials 32 32
Continue to review full report at Codecov.
|
|
TODO:
|
|
Tests for unfixed issues are still marked as $ python setup.py run_all_tests ... |
|
I'll merge this PR tomorrow (UTC+09:00). If you have comments or feedback, leave comments by then. |
2 tasks
…tests * Improve the usage of event loops in unit tests not to be affected by other tests * Add pytest.ini for detailed logging * Add `python setup.py run_all_tests --test-target web` * Add integration tests both for async/sync operations, including the reproducing case for slackapi#569 * Move samples from tests to integration_tests as they require real tokens
Now it's possible run `python setup.py run_all_tests --integration-test-target=web/test_issue_654.py`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request improves the test settings and adds integration tests with real Slack APIs. This is a preparation for my forthcoming pull requests addressing the concurrency issues in this library.
python setup.py run_all_tests --test-target webIntegration tests
The integration tests under the
integration_testsdirectory are supposed to be executed manually by the maintainers. They won't be integrated into the CI builds at least in the short-term.Env variables for integration_tests
The integration tests require real tokens and channel IDs to access in the tests.
New setuptools command -
run_all_testsA new setuptools command named
run_all_testsis a handy way to run all the unit tests and integration tests. You can filter the tests to run by the--test-targetoption.`python setup.py run_all_tests` outputs
Requirements (place an
xin each[ ])