Use threads instead of processes in test_pocs.py #468
Conversation
Run pocs in a thread instead of in a separate process to make it easier to share resources between the test thread and pocs. This was motivated by PanMemoryDB, but I think it will help in general. Add a Timeout class to pocs.utils and use in test_pocs to force the ending of long running tests. Shutdown pocs even if message not received; use a try-finally block to ensure we send the shutdown message.
Codecov Report
@@ Coverage Diff @@
## develop #468 +/- ##
===========================================
- Coverage 67.64% 67.53% -0.12%
===========================================
Files 63 63
Lines 5306 5328 +22
Branches 737 740 +3
===========================================
+ Hits 3589 3598 +9
- Misses 1520 1528 +8
- Partials 197 202 +5
Continue to review full report at Codecov.
|
pocs/tests/test_pocs.py
Outdated
| return False | ||
|
|
||
|
|
||
| def wait_for_pointing(sub, max_duration=90): |
There was a problem hiding this comment.
The MemoryDB PR has a note about perhaps changing which state we check for so we don't run through scheduling.
There was a problem hiding this comment.
Yup, I've incorporated that in this branch, pushing in a few minutes.
| if self.is_non_blocking: | ||
| return 0 | ||
| else: | ||
| delta = self.target_time - time.monotonic() |
There was a problem hiding this comment.
Am I missing something about where these come from? Should this class inherit from serialutil.Timeout?
There was a problem hiding this comment.
I decided to just copy their code and eliminate things we don't need. time.monotonic() is like time.time(), but guarantees that it won't go backwards.
There was a problem hiding this comment.
Ok, I didn't realize target_time first gets set in the restart method so didn't see where it was coming from. Makes sense now.
There was a problem hiding this comment.
Although maybe just start is a better name?
There was a problem hiding this comment.
restart because init calls it, so that it is started at creation. If too confusing, I could remove restart, so you have to create a new instance each time, which is definitely the common way to use it.
| class Timeout(object): | ||
| """Simple timer object for tracking whether a time duration has elapsed. | ||
|
|
||
| Attribute `is_non_blocking` is true IFF the duration is zero. |
There was a problem hiding this comment.
I don't understand where non-blocking case. Where would you be using a timeout=0?
There was a problem hiding this comment.
Yes, timeout==0 means that it will always be considered to have expired.
|
This change is really about simplifying #463 which had gotten too large, reflecting the various bugs I found along the while debugging use of the memory db. |
Use hardware module to get the list of simulator names, minus 'weather'. Wait for pocs thread to stop even if an assertion has failed, to avoid having two running at once (i.e. if the next test starts while the previous test's thread is still running).
| if self.is_non_blocking: | ||
| return 0 | ||
| else: | ||
| delta = self.target_time - time.monotonic() |
There was a problem hiding this comment.
Ok, I didn't realize target_time first gets set in the restart method so didn't see where it was coming from. Makes sense now.
| if self.is_non_blocking: | ||
| return 0 | ||
| else: | ||
| delta = self.target_time - time.monotonic() |
There was a problem hiding this comment.
Although maybe just start is a better name?
Run pocs in a thread instead of in a separate process to make it easier
to share resources between the test thread and pocs. This was motivated
by PanMemoryDB, but I think it will help in general.
Add a Timeout class to pocs.utils and use in test_pocs to force the
ending of long running tests.
Shutdown pocs even if message not received; use a try-finally block
to ensure we send the shutdown message.