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

Add time.set_timer() unittest #1924

Merged
merged 6 commits into from Jun 8, 2020
Merged

Add time.set_timer() unittest #1924

merged 6 commits into from Jun 8, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jun 5, 2020

I tired to do a unittest for set_timer.
I wanted to test the time between events individually, but it turns out the firsts calls to pygame.event.get() are slower than the others for some reason. Some of them are randomly slow too. The number of events doesn't seem to really affect it.

import pygame

pygame.init()

test_event = pygame.event.Event(pygame.event.custom_type())
for i in range(10):
    pygame.event.post(test_event)
    t1 = pygame.time.get_ticks()
    event_ammount = len(pygame.event.get())
    t2 = pygame.time.get_ticks()
    print("{} ms to get {} events".format(t2-t1, event_ammount))
319 ms to get 4 events
93 ms to get 1 events
0 ms to get 1 events
0 ms to get 1 events
0 ms to get 1 events
0 ms to get 1 events
0 ms to get 1 events
0 ms to get 1 events
0 ms to get 1 events
0 ms to get 1 events
  • Instead, I wait for a number of events and look for the total time the test takes. I guess it's good enough?

  • I also use a 250ms delay with 8 events (2 seconds) to be sure the first call to get() won't take more time than the whole test.

@ghost ghost mentioned this pull request Jun 5, 2020
@ghost
Copy link
Author

ghost commented Jun 5, 2020

Should I start by pumping the events a few times before starting the test to reduce the event.get() initial delay I mentioned earlier? This way the test could be more reliable. But it's far from perfect...

As I said, it doesn't test the individual events timing, but I don't know how I could test it, unless I use a really long test time to prevent errors. Otherwise, the firsts events are all posted in the delay of the first get().

I also thought I could use the timestamp of the SDL events, but it seems they are not used in pygame.

@ghost
Copy link
Author

ghost commented Jun 6, 2020

So, it turns out, the initial delay in pygame.event.get is actually in SDL_PumpEvents.

for (i = 0; i < 10; i++) {
    t1 = SDL_GetTicks();
    SDL_PumpEvents();
    t2 = SDL_GetTicks();
    printf("%i\n", t2-t1);
}
322
5
15
0
0
0
0
0
0
0

So, even if I think it's ugly, I changed the test to call pygame.event.get() a few times before starting the test. It should reduce the risk of the first call taking too much time and make the test fail.

I also increased the delta to 200ms (∓ 10% of the test). It looks like time stuff is somewhat variable, especially on CI tools. At 100ms, one random Windows build would occasionally fail. I guess it should be fine now.

@MyreMylar
Copy link
Contributor

MyreMylar commented Jun 8, 2020

I expect pump takes longer in the first few calls near the beginning because there is probably a backlog of SDL/platform events that get created during the initialisation process.

Copy link
Contributor

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

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

LGTM, seems like you are testing everything :thumbs:

I think for the time module error deltas on CI we'll only find out over time if the set values are workable as the performance seems quite variable.

@MyreMylar MyreMylar merged commit b512bf5 into pygame:master Jun 8, 2020
@ghost ghost deleted the set_timer-unittest branch June 8, 2020 20:27
@illume illume added the time pygame.time label Sep 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
time pygame.time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants