dpytest icon indicating copy to clipboard operation
dpytest copied to clipboard

Efficiently test task loops

Open ctmbl opened this issue 1 year ago • 8 comments

EDIT: just correcting a misspelling: dpytest.run_all_events -> dpytest.run_all_loops

Is your feature request related to a problem? Please describe.

I'd like to efficiently test a cog's task loop. The documentation is lacking an example if this is already possible in some ways.

Describe the solution you'd like

I have a bot that load a cog. This cog has 2 commands, that I easily managed to test using dpytest, and 1 task loop. One of these commands adds a user. The task loop that regularly checks if all users' state have changed (it fetches an API (mocked)). Here is how it should happend:

  • at the beginning there is no user the loop shouldn't send any message
  • I send the command !adduser 1, it adds the user
  • the loop should now fetch the user 1 state from the mocked API and send a message in the channel
  • I dpytest.verify that the last message sent was about this new user state --> the loop has done its job

a way to manually trigger the loop, like ~~dpytest.run_all_events()~~dpytest.run_all_loops would be great I think.

Describe alternatives you've considered

I didn't considered any, I also don't know if that's currently possible

ctmbl avatar Jun 06 '23 13:06 ctmbl

what happens if your run run_all_events by hand ?

Sergeileduc avatar Jun 06 '23 14:06 Sergeileduc

@Sergeileduc If "by hand" you mean in a python interpreter, then I don't know. But I tried to run it in my test and it seems not to trigger the loop. Also I start the loop in the cog's __init__ so that's not the problem

ctmbl avatar Jun 06 '23 22:06 ctmbl

Ho, sorry @ctmbl , I must have been really tired the other day.

Yeah ok, I see what you mean.

yeah, that's a good idea !

as you said, dpytest.run_all_events() could be great.

I don't know if I can, or have time, but it could be a great feature.

so not for now, but I'll try to dig, someday.

Sergeileduc avatar Jun 16 '23 14:06 Sergeileduc

@Sergeileduc

Tks for your interest!

Actually I noticed I misspelled the function I'd like to see implemented: dpytest.run_all_loops (dpytest.run_all_events already exists and doesn't trigger the loops). I updated the original message sorry for the confusion :smiling_face_with_tear:

I find a workaround but it is a really bad one... I get the task object from the bot, then modify the delay between each iteration to an arbitrary small value (10 ms), and finally wait 15ms so that the loop has been triggered (see the code). But this is extremely dirty and not reliable so I hope you'll manage to find some time to implement such a dpytest.run_all_loops, I could also propose a PR myself if you'd like!

ctmbl avatar Jun 22 '23 09:06 ctmbl

sure ! feel free to PR, because I didn't start to work on that, so, yeah, if you allready has the begining of something, go for the PR 👍

I don't know how the loops work, in discord.py (internally, I mean) but using "change_interval" doesn't look super dirty too me. (I was thinking "if loops use asyncio.sleep or something like that, we will have to monkeypatch the sleep, etc... 😂. So, in comparison, your approach with change_interval looks pretty decent)

I don't know how we can manage too hide all this in a dpytest.run_all_loops function, however

maybe we don't have too, but just add some documentation, on how to use change_interval ? IDK, actually

if you have any idea, feel free to PR something

Sergeileduc avatar Jun 22 '23 14:06 Sergeileduc

et t'es français non ? (fin on va continuer en anglais quand même)

Sergeileduc avatar Jun 22 '23 14:06 Sergeileduc

@Sergeileduc

et t'es français non ? (fin on va continuer en anglais quand même)

ahah exact, je le prends un peu mal mon anglais est si mauvais ? :laughing:

sure ! feel free to PR, because I didn't start to work on that, so, yeah, if you allready has the begining of something, go for the PR +1

Not for now, and I won't be able to dig in before next week but was just asking :wink:

maybe we don't have too, but just add some documentation, on how to use change_interval ? IDK, actually

We could actually. But the best would be for sure to "destroy" the loop and then trigger the callback it "by hand".

What is dirty in my change_interval is actually the asyncio.sleep that follows it. First it really slows down the test, second it isn't reliable at all because it is arbitrary. I can't say for sure that in 15ms my loop will be triggered AND done. To ensure reliability I must increase the sleep time and then it slows down even more the test, you got it it's a bad compromise.

ctmbl avatar Jun 23 '23 08:06 ctmbl

je le prends un peu mal mon anglais est si mauvais ? 😆

ha ha, pas du tout, c'est juste le nom ! 😂

Sergeileduc avatar Jun 23 '23 14:06 Sergeileduc