Conversation
Add 13 unit tests for the ASGI integration layer covering environ building, body reading, start response, sender, lifespan handling and invalid scope rejection. Add conftest.py to skip ASGI test collection on Python < 3.5 where async syntax causes SyntaxError. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds comprehensive ASGI unit tests for the Appier framework and refines ASGI handling logic. Changes include a new test suite with ASGI-WSGI bridging tests, lifespan startup/shutdown failure handling per ASGI specification, and query string decoding corrected from ASCII to Latin-1 per PEP 3333. Changes
Sequence Diagram(s)sequenceDiagram
participant Receiver as ASGI Receiver
participant Manager as Lifespan Manager
participant Sender as ASGI Sender
Receiver->>Manager: lifespan.startup event
rect rgba(100, 200, 100, 0.5)
alt Startup Success
Manager->>Manager: start() app
Manager->>Sender: lifespan.startup.complete
else Startup Failure
Manager->>Manager: exception caught
Manager->>Sender: lifespan.startup.failed
end
end
Receiver->>Manager: lifespan.shutdown event
rect rgba(100, 150, 200, 0.5)
alt Shutdown Success
Manager->>Manager: stop() app
Manager->>Sender: lifespan.shutdown.complete
else Shutdown Failure
Manager->>Manager: exception caught
Manager->>Sender: lifespan.shutdown.failed
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/appier/test/conftest.py`:
- Around line 33-34: The version-check logic using sys.version_info[0] < 3 or
sys.version_info[1] < 5 is incorrect for future major versions; update the
condition to a tuple comparison such as sys.version_info[:2] < (3, 5) so that
the check correctly evaluates versions (this affects the existing collect_ignore
append("asgi.py") branch referencing sys.version_info and collect_ignore).
🧹 Nitpick comments (2)
src/appier/test/conftest.py (1)
5-5: Copyright year may need updating.The copyright shows 2008-2024, but we're currently in 2026. Consider updating to 2008-2026 for consistency.
src/appier/test/asgi.py (1)
5-5: Copyright year may need updating.Same as conftest.py, the copyright shows 2008-2024 but should likely be 2008-2026.
src/appier/test/conftest.py
Outdated
| if sys.version_info[0] < 3 or sys.version_info[1] < 5: | ||
| collect_ignore.append("asgi.py") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the conftest.py file exists and read it
cat -n src/appier/test/conftest.pyRepository: hivesolutions/appier
Length of output: 1538
🏁 Script executed:
# Check for asgi.py in the test directory and elsewhere
fd -t f "asgi.py" src/appier/Repository: hivesolutions/appier
Length of output: 126
🏁 Script executed:
# Search for imports of asgi module
rg "from.*asgi import|import.*asgi" src/appier/ --glob "!**/asgi.py"Repository: hivesolutions/appier
Length of output: 105
Fix version check logic for future Python compatibility.
The condition sys.version_info[0] < 3 or sys.version_info[1] < 5 uses OR logic which fails for hypothetical Python 4.x versions with minor < 5 (e.g., Python 4.1 would incorrectly exclude asgi.py tests). Use tuple comparison for robust version checking:
-if sys.version_info[0] < 3 or sys.version_info[1] < 5:
+if sys.version_info[:2] < (3, 5):
collect_ignore.append("asgi.py")🤖 Prompt for AI Agents
In `@src/appier/test/conftest.py` around lines 33 - 34, The version-check logic
using sys.version_info[0] < 3 or sys.version_info[1] < 5 is incorrect for future
major versions; update the condition to a tuple comparison such as
sys.version_info[:2] < (3, 5) so that the check correctly evaluates versions
(this affects the existing collect_ignore append("asgi.py") branch referencing
sys.version_info and collect_ignore).
There was a problem hiding this comment.
Pull request overview
Adds unit test coverage for the ASGI integration layer while preventing pytest from collecting async/await tests on unsupported Python versions.
Changes:
- Added 13 unit tests validating ASGI environ building, body reading, start response behavior, sender behavior, lifespan handling, and invalid scope rejection.
- Added a pytest
conftest.pycollection guard to avoid SyntaxError on interpreters that can’t parseasync/await. - Updated
CHANGELOG.mdto record the added ASGI tests and collection guard.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/appier/test/conftest.py | Adds pytest collection ignore logic for ASGI test module on older Python versions. |
| src/appier/test/asgi.py | Introduces a new unittest suite covering ASGI helper methods and lifespan handling. |
| CHANGELOG.md | Documents the addition of ASGI unit tests and the collection guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/appier/test/conftest.py
Outdated
| # ignores test modules that use async/await syntax and would | ||
| # cause a SyntaxError when collected under Python 2.7 | ||
| collect_ignore = [] | ||
| if sys.version_info[0] < 3 or sys.version_info[1] < 5: |
There was a problem hiding this comment.
The version guard is using sys.version_info[0] < 3 or sys.version_info[1] < 5, which can misclassify versions (e.g., Python 4.0 would still match because 0 < 5). Use a tuple comparison like sys.version_info < (3, 5) (or an explicit major/minor check) so the ignore logic remains correct for future versions.
| if sys.version_info[0] < 3 or sys.version_info[1] < 5: | |
| if sys.version_info < (3, 5): |
src/appier/test/asgi.py
Outdated
| import asyncio | ||
| import unittest | ||
|
|
||
| import appier | ||
|
|
||
|
|
||
| class ASGITest(unittest.TestCase): | ||
| def setUp(self): | ||
| self.app = appier.App() | ||
|
|
There was a problem hiding this comment.
These tests exercise ASGI helpers that currently call asyncio.create_task (see src/appier/asgi.py), which is unavailable on Python 3.5/3.6. Since the repo CI matrix includes 3.5 and 3.6, this file will likely fail at runtime on those versions unless you either (a) skip the whole module/class when asyncio.create_task is missing, or (b) update the ASGI implementation to fall back to asyncio.ensure_future/loop.create_task and keep the tests enabled.
|
|
||
| * | ||
| * ASGI unit tests covering environ building, body reading, start response, sender, lifespan and invalid scope handling | ||
| * Test conftest to exclude ASGI tests on Python < 3.5 |
There was a problem hiding this comment.
The changelog entry says ASGI tests are excluded on Python < 3.5, but the new ASGI tests will still run on 3.5/3.6 (and currently the ASGI implementation uses asyncio.create_task, which isn’t available there). After deciding whether to skip on <3.7 or add a compatibility fallback, please update this line to reflect the actual supported/covered versions so the changelog doesn’t mislead.
| * Test conftest to exclude ASGI tests on Python < 3.5 | |
| * Test conftest to exclude ASGI tests on Python < 3.7 |
…mpat Move ASGI test implementation from appier.test._asgi_async to appier._asgi_test so setuptools' test loader does not discover and import it on Python 2.7 (where async syntax causes SyntaxError). Remove conftest.py as it only affected pytest which requires 3.7+. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add try/except to asgi_lifespan() so startup/shutdown exceptions send lifespan.startup.failed or lifespan.shutdown.failed events with a message field, as required by the ASGI lifespan spec. Change query_string decoding from ASCII to latin1 in _build_environ() to comply with PEP 3333 and avoid UnicodeDecodeError on non-ASCII bytes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/appier/test_py3/_asgi_test.py`:
- Around line 1-33: The file src/appier/test_py3/_asgi_test.py is failing Black
formatting; reformat the module with Black (e.g., black . or black
src/appier/test_py3/_asgi_test.py) so the imports and top-level metadata (the
__author__, __copyright__, __license__ declarations and the import block: io,
asyncio, unittest) conform to the project's Black settings, then stage the
formatted file and update the PR.
🧹 Nitpick comments (1)
src/appier/asgi.py (1)
141-171: Avoid swallowing task cancellation in lifespan error handling.Catching
Exceptionwill interceptasyncio.CancelledErroron some Python versions, preventing proper cancellation. Re-raise cancellation before emitting failed events.Proposed adjustment
if event["type"] == "lifespan.startup": try: if not self.is_started(): self.start() await send(dict(type="lifespan.startup.complete")) - except Exception as exception: + except asyncio.CancelledError: + raise + except Exception as exception: await send( dict( type="lifespan.startup.failed", message=str(exception), ) ) break elif event["type"] == "lifespan.shutdown": try: if not self.is_stopped(): self.stop() await send(dict(type="lifespan.shutdown.complete")) - except Exception as exception: + except asyncio.CancelledError: + raise + except Exception as exception: await send( dict( type="lifespan.shutdown.failed", message=str(exception), ) )
| #!/usr/bin/python | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| # Hive Appier Framework | ||
| # Copyright (c) 2008-2024 Hive Solutions Lda. | ||
| # | ||
| # This file is part of Hive Appier Framework. | ||
| # | ||
| # Hive Appier Framework is free software: you can redistribute it and/or modify | ||
| # it under the terms of the Apache License as published by the Apache | ||
| # Foundation, either version 2.0 of the License, or (at your option) any | ||
| # later version. | ||
| # | ||
| # Hive Appier Framework is distributed in the hope that it will be useful, | ||
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| # Apache License for more details. | ||
| # | ||
| # You should have received a copy of the Apache License along with | ||
| # Hive Appier Framework. If not, see <http://www.apache.org/licenses/>. | ||
|
|
||
| __author__ = "João Magalhães <joamag@hive.pt>" | ||
| """ The author(s) of the module """ | ||
|
|
||
| __copyright__ = "Copyright (c) 2008-2024 Hive Solutions Lda." | ||
| """ The copyright for the module """ | ||
|
|
||
| __license__ = "Apache License, Version 2.0" | ||
| """ The license for the module """ | ||
|
|
||
| import io | ||
| import asyncio | ||
| import unittest |
There was a problem hiding this comment.
Black formatting check is failing for this file.
CI indicates one file needs Black formatting—please run Black on this module.
As per coding guidelines, Format Python code with Black before committing.
🤖 Prompt for AI Agents
In `@src/appier/test_py3/_asgi_test.py` around lines 1 - 33, The file
src/appier/test_py3/_asgi_test.py is failing Black formatting; reformat the
module with Black (e.g., black . or black src/appier/test_py3/_asgi_test.py) so
the imports and top-level metadata (the __author__, __copyright__, __license__
declarations and the import block: io, asyncio, unittest) conform to the
project's Black settings, then stage the formatted file and update the PR.
Add 13 unit tests for the ASGI integration layer covering environ building, body reading, start response, sender, lifespan handling and invalid scope rejection. Add conftest.py to skip ASGI test collection on Python < 3.5 where async syntax causes SyntaxError.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests