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 __str__ to TaskRunnable in conds #149

Merged
merged 3 commits into from
Nov 27, 2022
Merged

Conversation

egisxxegis
Copy link
Contributor

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.

@egisxxegis
Copy link
Contributor Author

egisxxegis commented Nov 21, 2022

Link to issue: issue

@Miksus
Copy link
Owner

Miksus commented Nov 23, 2022

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.

@egisxxegis
Copy link
Contributor Author

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).

pull1
...
pull2

@codecov-commenter
Copy link

Codecov Report

Base: 94.90% // Head: 95.05% // Increases project coverage by +0.14% 🎉

Coverage data is based on head (da8f881) compared to base (41b48bd).
Patch coverage: 84.61% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #149      +/-   ##
==========================================
+ Coverage   94.90%   95.05%   +0.14%     
==========================================
  Files          80       80              
  Lines        4454     4467      +13     
==========================================
+ Hits         4227     4246      +19     
+ Misses        227      221       -6     
Impacted Files Coverage Δ
rocketry/conditions/api.py 97.34% <60.00%> (-1.03%) ⬇️
rocketry/conditions/task/task.py 98.03% <100.00%> (+4.16%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Miksus
Copy link
Owner

Miksus commented Nov 27, 2022

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!

@Miksus Miksus merged commit 5046597 into Miksus:master Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants