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

Fix tests and cache containers for CI #160

Merged
merged 16 commits into from
Feb 7, 2024
Merged

Fix tests and cache containers for CI #160

merged 16 commits into from
Feb 7, 2024

Conversation

eholum
Copy link
Collaborator

@eholum eholum commented Feb 7, 2024

Replacing: #159, since I have absolute power now.

Resolves: #85

This PR:

  • Fixes one unit test
  • Ensures that the run_tests.sh script returns an accurate failure code if tests fail
  • Adds caching to/from Dockerhub for the ROS builds to speed up dependency build times

Copy link

github-actions bot commented Feb 7, 2024

Coverage

Coverage Report
FileStmtsMissCoverMissing
__init__.py00100% 
core
   __init__.py90100% 
   dynamics.py430100% 
   gazebo.py136894%55, 124, 230, 244–248
   hallway.py731086%59, 61, 63, 96, 158, 196–199, 205
   locations.py1262183%53, 55, 69, 84–85, 87, 98–99, 101–102, 119, 177, 192, 217, 227, 273, 284–285, 287–288, 312
   objects.py69494%78, 106, 137, 197
   robot.py2145872%140–141, 203, 216, 263, 277–278, 282, 286, 292–293, 295–296, 313–314, 342, 344–345, 366–367, 370–373, 376–378, 403, 407–409, 411, 413–416, 429–431, 435, 437–439, 441, 444–445, 448, 470–471, 479, 484–489, 492, 505
   room.py48197%113
   world.py57113875%146–147, 183–184, 188, 241–242, 244–245, 273–274, 325, 339–340, 342–343, 372–376, 378–380, 382–383, 386, 390–394, 397–399, 402–409, 440, 476–477, 480, 496, 526–527, 538, 541, 563–567, 569–571, 574–575, 578–581, 583, 586, 588–590, 592–594, 596, 615, 625–629, 664–665, 668–671, 688–689, 695–696, 698–699, 701, 703–704, 706, 708–709, 719–720, 723, 725, 739, 741, 748, 795, 826–827, 846–847, 851–852, 887–888, 890, 907–908, 927–931, 943–946, 958–961, 986–987, 989, 1033–1034, 1166–1167
   yaml_utils.py1061189%64, 68, 152–154, 158–159, 182–183, 207–208
gui
   __init__.py20100% 
   main.py1522980%22, 24–26, 28, 49, 178–181, 188–191, 193, 196–202, 206–208, 212–213, 219, 230
   world_canvas.py2265276%34–36, 110, 112, 114, 177, 181, 232–237, 243–244, 246–248, 253–254, 257, 260–262, 265, 270–271, 273, 300, 322–327, 330, 333–334, 349–351, 354, 371, 400, 418–419, 421–422, 464, 480, 487
manipulation
   __init__.py10100% 
   grasping.py2366174%102, 110–112, 114–122, 124, 132, 462, 464–466, 470–472, 566–568, 571–575, 583, 586–588, 593–594, 600–604, 607–614, 617–619, 628, 630, 632–638
navigation
   __init__.py40100% 
   a_star.py781087%67–72, 74–76, 165
   execution.py34488%46–47, 49–50
   occupancy_grid.py1391489%44–47, 49–53, 56, 192, 226, 234, 238
   path_planner.py26676%32, 35, 37, 41, 84, 89
   planner_base.py591771%29–35, 51, 57–59, 140–145
   prm.py725622%40–44, 46, 50–51, 54, 57–64, 66–67, 76–79, 85, 101–102, 104–109, 111–112, 115–118, 121–126, 135, 144, 154, 156, 158–159, 161, 174–178
   rrt.py172795%228, 353–354, 360–361, 363, 392
   trajectory.py500100% 
   world_graph.py70395%118, 121, 145
planning
   __init__.py30100% 
   actions.py664334%59–60, 62, 66–75, 77–80, 82–86, 88–91, 93–97, 99–100, 102–104, 150–151, 154–158
planning/pddlstream
   __init__.py00100% 
   default_mappings.py90100% 
   planner.py40880%119–126
   primitives.py57296%68, 208
   utils.py85396%66, 99–100
utils
   __init__.py00100% 
   general.py33487%23, 45, 51–52
   knowledge.py1391688%90–91, 100, 135–138, 145, 149–150, 163, 178, 182, 224, 240, 295
   motion.py60690%56, 67–71
   polygon.py1110100% 
   pose.py89396%79, 211, 227
   search_graph.py93396%241–243
TOTAL350159882% 

Tests Skipped Failures Errors Time
163 0 💤 0 ❌ 0 🔥 3m 57s ⏱️

eholum and others added 2 commits February 7, 2024 09:22
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
@sea-bass
Copy link
Owner

sea-bass commented Feb 7, 2024

Well, this saved a good chunk of time for the ROS tests!

There's probably a way to also make a Docker image for the ROS-free version so that the PDDLStream building is similarly cached... but definitely not necessary.

I'm good to approve this whenever you tell me it's ready, @eholum !

@eholum
Copy link
Collaborator Author

eholum commented Feb 7, 2024

Well, this saved a good chunk of time for the ROS tests!

Woohoo!

There's probably a way to also make a Docker image for the ROS-free version so that the PDDLStream building is similarly cached... but definitely not necessary.

I started down this road by adding a new non-ros target to the dockerfile but probably don't have the strength of will to tackle that as part of this PR.

I'm good to approve this whenever you tell me it's ready, @eholum !

Right now this is going to cache and push everything to a single "main" image. Probably not a huge deal until someone starts pulling and using images instead of building locally, but as long as your compose file doesn't reference what you're using in CI it won't matter for users. For CI, having multiple PRs open at once and you'll start stepping on each other's toes. We could tag with branch names at the cost of having to occasionally clear out the registry. Do you have a preference?

@sea-bass
Copy link
Owner

sea-bass commented Feb 7, 2024

I don't think this repo is active enough / the dependencies will change frequently enough to do per branch caching at this moment.

So I'm OK keeping it as is for now.

@eholum eholum marked this pull request as ready for review February 7, 2024 15:20
@eholum eholum changed the title [WIP] Fix and speed up CI Fix tests and cache containers for CI Feb 7, 2024
@sea-bass sea-bass merged commit c43ff3e into main Feb 7, 2024
12 of 16 checks passed
@sea-bass sea-bass deleted the speed-up-coverage branch February 9, 2024 13:30
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.

Improve automated tests to use docker images.
2 participants