simple-pid icon indicating copy to clipboard operation
simple-pid copied to clipboard

Ensure tests are deterministic and faster

Open gonzalo-bulnes opened this issue 1 year ago • 1 comments

Overview

This PR does two related things in three steps:

  • Fix a small oversight in the comparison of elapsed time to sampling time.
  • Introduce the use of simulated time in the test suite, which enables:
    • deterministic tests
    • faster tests (full run under a second)
  • Take a temporary step to optimize waiting time in tests. That becomes irrelevant after the move to simulated time, but I thought it was interesting to put the speed-up numbers in perspective.

It also suggests the removal of some code, because the time_fn option makes the dt option redundant (as demonstrated by the use of simulated time in the test suite). Context in #10. (:warning: That's very much YMMV, admittedly, and I can't judge if removing it breaks backwards-compatibility, it's your definition that matters, not mine. See commit message for more details.)

:crystal_ball: There are extensive comments in the commit messages.

Fixes #70

Implementation

I happen to have written a drop-in replacement for time.time() and time.monotonic() some time ago, that provides simulated time for testing purposes. (stime on GitHub, and PyPi)

The introduction of the time_fn option to inject the time function enables the best case scenario to introduce stime.

This is the first time I actually use this little package, and I'm pretty happy with how it fits this project. (Fun fact: I wrote as a simple pretext to learn how to write a Python package, at a time I thought I'd need it eventually when implementing a PID, before I found this package. So the projects were somehow related in my mind from the start. Needless to say I never ended up writing that PID package.)

The general idea is:

  1. import stime
  2. (Re-)set the clock to whatever time (0 is as good a choice as any other Unix epoch) with stime.reset(0).
  3. Whenever useful, advance time by one second stime.tick() or any arbitrary value stime.tick(0.1), or reset it at will.

That's it.

Outcome

Unless I'm misunderstanding something about how the PID works, the current test suite should now be deterministic.

The overall speed of the test suite has increased significantly. For reference, an entire test suite run on my machine went from 2m14, to 26s, to 200ms. That's a roughly 600x speed-up, of which I'd consider the last 100x factor meaningful.

I also think that tests with simulated time are easier to reason about, because the test code itself doesn't affect the timings. (That's another effect of the same property that enables writing deterministic tests.)

Checklist

  • [x] Tests
  • [x] Linting and formatting
  • [x] Docs / explanations

gonzalo-bulnes avatar Oct 02 '23 10:10 gonzalo-bulnes