-
Notifications
You must be signed in to change notification settings - Fork 152
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
add advance_time fixture and test (closes #83, #95, #96 #110) #113
base: main
Are you sure you want to change the base?
add advance_time fixture and test (closes #83, #95, #96 #110) #113
Conversation
* master: Simplified fixture setup by relying on the fact that get_event_loop returns the loop provided by the event_loop fixture of pytest-asyncio. Removed a layer of indirection when checking for markers. There is only one marker and one fixture provided by pytest-asyncio. Retrieving these from a dictionary seems unnecessary complicated. Add test for hypothesis event_loop reuse. Sync wrapper for hypothesis tests uses the event loop provided by pytest-asyncio instead of creating a new loop. 0.11 open for business. Actually bump version. Changelog. Make transfer_markers a noop when importing fails Revert "Remove transfer_markers" Remove transfer_markers More specific Hypothesis detection Support async tests which use Hypothesis Move pytest warning config to setup.cfg Fix: Avoid warning on latest Pytest versions
* advance_time-fixture: add more tests to verify the advance_time fixture operations simplify EventLoopClockAdvancer and clarify __call__ method Simplified fixture setup by relying on the fact that get_event_loop returns the loop provided by the event_loop fixture of pytest-asyncio. Removed a layer of indirection when checking for markers. There is only one marker and one fixture provided by pytest-asyncio. Retrieving these from a dictionary seems unnecessary complicated. Add test for hypothesis event_loop reuse. Sync wrapper for hypothesis tests uses the event loop provided by pytest-asyncio instead of creating a new loop. foo
I went through and added some additional tests. Also, I think the |
I just tried this in a Python 3.7 project I'm working on with the following code: async def race(
futures: Iterable[FutureT[T]],
*,
timeout: Optional[float] = None,
loop: Optional[asyncio.AbstractEventLoop] = None,
) -> T:
if loop is None:
loop = asyncio.get_running_loop()
tasks = {future for future in futures}
done, pending = await asyncio.wait(
tasks, timeout=timeout, return_when=asyncio.FIRST_COMPLETED, loop=loop
)
try:
if len(pending) == len(tasks):
raise asyncio.TimeoutError()
return done.pop().result()
finally:
for future in pending:
future.cancel() My test for this function looks like this: @pytest.mark.asyncio
async def test_race(event_loop, advance_time):
async def one():
await asyncio.sleep(100, loop=event_loop)
return 1
async def two():
await asyncio.sleep(50, loop=event_loop)
return 2
async def three():
await asyncio.sleep(25, loop=event_loop)
return 3
task = event_loop.create_task(race([one(), two(), three()], loop=event_loop))
await advance_time(35)
await advance_time(60)
await advance_time(110)
assert task.result() == 3 I copied the implementation in this PR into my async def __call__(self, seconds):
"""
Advance time by a given offset in seconds. Returns an awaitable
that will complete after all tasks scheduled for after advancement
of time are proceeding.
"""
# sleep so that the loop does everything currently waiting
await asyncio.sleep(0.0001)
if seconds > 0:
# advance the clock by the given offset
self.offset += seconds
# Once the clock is adjusted, new tasks may have just been
# scheduled for running in the next pass through the event loop
await asyncio.sleep(0.0001) |
Thanks for testing, @bryanforbes . What OS and loop policy are you loop using? I recently found that using |
I should mention for future reference, there is nothing (to my knowledge) we can do to fix this on |
@bryanforbes , you motivated me to add something I had been considering. I set the default to |
- for more simplicity the advnace-time-sleep has simply been changed to 1e-6 so that it works on most systems. - remove debug code
I'm on macOS 10.14.1 using the standard event loop (as far as I'm aware). I updated the code in my |
Hi folks, I'm thrilled to report that this fixture works as intended with my work codebase. Until a new release is cut out with this PR, I've vendored the fixture. This will allow us to progressively migrate our large body of tests written using asynctest (especially code using It's worth mentioning here (for the record) that In any case, let me know if there's anything I can do to help making sure this PR lands. Cheers. |
Hi folks, is there anything I can do to help get the ball rolling on this PR? We've been using it successfully for a few months now, it's rock solid and works as advertised. Thanks |
Hi folks, I've been revisiting old TODOs at work and this issue popped up. I was thinking, to help merging this PR, maybe an "experimental" disclaimer/tag in the doc could be added to this fixture so that neither its API nor its behavior is set in stone. Could this help pave the way forward? Maybe I should create a pytest-asyncio-advance-time package containing this fixture? (that is, until it is eventually merged, I really don't want to get into the business of maintaining yet-another tiny package) I'd really like to avoid that as pytest-asyncio really seems like the natural place for this fixture. Any input from the maintainers would be greatly appreciated, thanks! |
BTW, I have a loop proxy implementation that allows the wallclock time tweaking without monkey-patching of the original event loop. |
This is reworking of the
advance_time
method from #96 and #95 into an event-loop independent fixture. This removes the need to change the event-loop class or policy on the fly. Compared to the fixture discussed in #83, this technique does not require any private method calls to the event-loop, which potentially volatile depending on the loop implementation. One thing to keep in mind, this technique requires patching the time method of the event loop, but the original base time method is stored to reduce the assumptions required about the timing methodology, which is left up to loop creators. This should be a fairly safe implementation.