Closed egisxxegis closed 1 year ago
Link to issue: issue
Thanks for the contribution, I think that looks good. Mind you creating a test case for this? I can also do it in case the test suites seem confusing, probably have time on Saturday for it.
Sorry for the delay in response.
I have just added the test, and additionally added __str__
for TimeCondWrapper (because the test was failing). I am adding two screenshots of current pytest results (I suspect it has nothing to do with added __str__
methods).
...
Base: 94.90% // Head: 95.05% // Increases project coverage by +0.14%
:tada:
Coverage data is based on head (
da8f881
) compared to base (41b48bd
). Patch coverage: 84.61% of modified lines in pull request are covered.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
Yep, I think so too :)
The tests are a bit dependent on how fast your system is. It's pretty hard to do tests that test race conditions without relying on execution times. And that means the tests can depend on how fast your system is.
The most important thing is that the tests work well on the CI.
But I think it looks good. Thanks for the contribution and the effort you saw to fix this!
This pull request adds str method to TaskRunnable, found in rocketry.conditions.task.task.TaskRunnable. By adding str method to TaskRunnable, we solve issue of AttributeError in issue (to be updated: I will create an issue) and let cast TaskRunnable to string.