-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Add code coverage, more end-to-end tests #56
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
Conversation
pre-commit = "^3.6.2" | ||
debugpy = "^1.8.1" | ||
pytest = "^8.1.0" | ||
pytest-cov = "^5.0.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we have something in README in ### Contribute
section indicating how to install dev dependencies? Are they installed when we do poetry install
the first time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are installed by default with poetry install
. They can be skipped with poetry install --without dev
.
Group dependencies are a conveniency offered by poetry for several purposes (one of which is staged build). In our case we just use it to differentiate those dev dependencies from the core ones (but it's really just labeling here)
("sim_transfer_cube", True, True), | ||
], | ||
) | ||
def test_aloha(task, from_pixels, pixels_only): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI took 12mn to run. It's too much. What could we do to parallelize? Not mandatory, but it would be nice to address this in this PR.
Once merged, dependencies will be cached so it should take ~4mns less.
Do you see any other params that could impact run time significantly? I will try those ideas before merging to see what works best. |
a253c45
to
404b8f8
Compare
I'm merging this with the eval ACT on Aloha (policy is None) test deactivated so that we can move on for now. |
…_03_27_improve_ci Add code coverage, more end-to-end tests
…ove_ci Add code coverage, more end-to-end tests
Note: I've split #54 into 2 PRs since they cover mainly unrelated changes
This adds:
test_aloha
intest_envs.py
which was missingTODO