-
Notifications
You must be signed in to change notification settings - Fork 235
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
Update Sanic adapter and its tests to be compatible with sanic v21 #432
Conversation
setup.py
Outdated
@@ -72,7 +72,8 @@ | |||
"Flask>=1,<2", | |||
"Werkzeug<2", # TODO: support Flask 2.x | |||
"pyramid>=1,<2", | |||
"sanic>=20,<21", | |||
"sanic>=21,<22", | |||
"sanic-testing>=0.6", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since sanic v21, the framework separated its testing support to a different PyPI package.
@@ -31,7 +31,8 @@ def to_sanic_response(bolt_resp: BoltResponse) -> HTTPResponse: | |||
resp.cookies[name]["expires"] = expire | |||
resp.cookies[name]["path"] = c.get("path") | |||
resp.cookies[name]["domain"] = c.get("domain") | |||
resp.cookies[name]["max-age"] = c.get("max-age") | |||
if c.get("max-age") is not None and len(c.get("max-age")) > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Sanic v21, an empty string may come here.
@@ -32,7 +32,7 @@ class TestSanic: | |||
|
|||
@staticmethod | |||
def unique_sanic_app_name() -> str: | |||
return f"awesome-slack-app-{time()}" | |||
return f"awesome-slack-app-{str(time()).replace('.', '-')}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix warnings since v21
@@ -107,7 +107,7 @@ async def endpoint(req: Request): | |||
|
|||
_, response = await api.asgi_client.post( | |||
url="/slack/events", | |||
data=body, | |||
content=body, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix deprecation warnings
|
||
# NOTE: Although sanic-testing 0.6 does not have this value, | ||
# Sanic apps properly generate the content-length header | ||
# assert response.headers.get("content-length") == "597" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented out due to the testing framework's behavior change
Codecov Report
@@ Coverage Diff @@
## main #432 +/- ##
=======================================
Coverage 91.36% 91.36%
=======================================
Files 167 167
Lines 5491 5492 +1
=======================================
+ Hits 5017 5018 +1
Misses 474 474
Continue to review full report at Codecov.
|
This pull request upgrades Sanic version for testing and fixes a few issues with the new major version.
Category (place an
x
in each of the[ ]
)slack_bolt.App
and/or its core componentsslack_bolt.async_app.AsyncApp
and/or its core componentsslack_bolt.adapter
/docs
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.
./scripts/install_all_and_run_tests.sh
after making the changes.