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

Upgrade test dependencies & fix Falcon warning #588

Merged
merged 2 commits into from Feb 4, 2022

Conversation

seratch
Copy link
Member

@seratch seratch commented Feb 4, 2022

This pull request upgrades the dependencies in testing. Also, it resolves a deprecation warning in Falcon 3.x.

Category (place an x in each of the [ ])

  • slack_bolt.App and/or its core components
  • slack_bolt.async_app.AsyncApp and/or its core components
  • Adapters in slack_bolt.adapter
  • Document pages under /docs
  • Others

Requirements (place an x in each [ ])

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

  • I've read and understood the Contributing Guidelines and have done my best effort to follow them.
  • I've read and agree to the Code of Conduct.
  • I've run ./scripts/install_all_and_run_tests.sh after making the changes.

@seratch seratch added enhancement New feature or request area:adapter labels Feb 4, 2022
@seratch seratch added this to the 1.11.5 milestone Feb 4, 2022
@seratch seratch self-assigned this Feb 4, 2022
Copy link
Member Author

@seratch seratch left a comment

Choose a reason for hiding this comment

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

comments for reviewers

setup.py Outdated
],
# pip install -e ".[testing_without_asyncio]"
"testing_without_asyncio": test_dependencies,
# pip install -e ".[testing]"
"testing": async_test_dependencies,
# pip install -e ".[adapter_testing]"
"adapter_testing": [
Copy link
Member Author

Choose a reason for hiding this comment

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

Organized the optional dependencies by moving test-related ones to this group

@@ -50,7 +50,11 @@ def _to_bolt_request(self, req: Request) -> BoltRequest:
)

def _write_response(self, bolt_resp: BoltResponse, resp: Response):
resp.body = bolt_resp.body
if falcon_version.__version__.startswith("2."):
resp.body = bolt_resp.body
Copy link
Member Author

Choose a reason for hiding this comment

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

response.text does not exist in 2.x

if falcon_version.__version__.startswith("2."):
resp.body = bolt_resp.body
else:
resp.text = bolt_resp.body
Copy link
Member Author

Choose a reason for hiding this comment

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

response.body is deprecated in 3.x

@@ -18,6 +20,13 @@
from tests.utils import remove_os_env_temporarily, restore_os_env


def new_falcon_app():
if falcon.version.__version__.startswith("2."):
return falcon.API()
Copy link
Member Author

Choose a reason for hiding this comment

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

falcon.API is deprecated in 3.x. However, the new recommended way falcon.App does not exist in 2.x

@codecov
Copy link

codecov bot commented Feb 4, 2022

Codecov Report

Merging #588 (b6fff44) into main (1d13969) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
- Coverage   91.53%   91.52%   -0.02%     
==========================================
  Files         169      169              
  Lines        5707     5709       +2     
==========================================
+ Hits         5224     5225       +1     
- Misses        483      484       +1     
Impacted Files Coverage Δ
slack_bolt/adapter/falcon/resource.py 82.92% <75.00%> (-1.69%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d13969...b6fff44. Read the comment docs.

@seratch
Copy link
Member Author

seratch commented Feb 4, 2022

👋 Let me know if you have any comments on this

Copy link
Member

@srajiang srajiang left a comment

Choose a reason for hiding this comment

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

Looks good!

@seratch
Copy link
Member Author

seratch commented Feb 4, 2022

Thanks for the review!

@seratch seratch merged commit 78b6487 into slackapi:main Feb 4, 2022
@seratch seratch deleted the upgrade-test-deps branch February 4, 2022 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:adapter enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants