-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Feat/migrate gps nav2 system test #4682
Feat/migrate gps nav2 system test #4682
Conversation
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
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.
I believe this will require the changes from the turtlebot minimum simulation package to be merged first (then we can add to our repos file to do here + run a sync so those changes will get out into binary format)
Signed-off-by: stevedan <[email protected]>
Yes, made the changes based on the review on the other PR |
@stevedanomodolor if you update this in your PR https://github.com/ros-navigation/navigation2/blob/main/tools/underlay.repos#L34-L37 to point at |
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
In this file, there are 3 instances of I thought I'd changed the underlay before and had it captured, but maybe vcstools doesn't consider the versions as a change 🤷 (at least I'm hoping so, else maybe there's a problem with this PR -- file naming?) |
I Doubt it is a problem with the file because I have verified that and the name is the same. It has something to do with the build tool and how the underws is built, not sure though the problem. |
Not really familiar with th build tool. What do you mean when you say v26 v27 |
There's 3 places in that file we name a cache and we need to break that cache by updating its name so that we pull and rebuild everything from scratch (lines 36, 41, 61). I think what's happening is that its still using the old underlay workspace without your updated files. If it still fails to run the test due to missing files at that point, its probably something in this PR with naming / file locations. |
Signed-off-by: stevedan <[email protected]>
@SteveMacenski Still does not work, I tried this locally and it works. I will look more into the cache things to see if we are missing something.
|
Signed-off-by: stevedan <[email protected]>
This passed, I'm confused as to the issue (unless that wasn't run in CI?) I'm seeing in the CI job that the min tb3 sim package is being built https://app.circleci.com/pipelines/github/ros-navigation/navigation2/12760/workflows/77329837-abfa-4597-90f3-c61db97f1b56/jobs/38504 in the 'build workspace /opt/underlay' step. The post checkout step says:
It looks like its behind your last commit? |
@ruffsl can you give us a hand here for a few minutes? |
Looks like the test case referenced above is passing: <testsuite name="nav2_system_tests" tests="1" failures="0" time="97.241" errors="0" skipped="0">
<testcase classname="nav2_system_tests" name="test_gps_waypoint_follower.missing_result" time="97.241"> </testcase>
</testsuite> Only this seemed to fail:
But perhaps is just being flaky: |
Rerunning - likely just a fluke! Thanks @ruffsl! I'm curious why changing from |
Umm, was the hash of that repos file with the diff set to main used before in this PR? If so, it may not have invalidated the cache, and restored from a previous cache. From a underlay cache perspective, these two subsequent commits look the same:
Currently a simple comparison is used to decide if the repos file has changed, and if a re-import is needed: navigation2/.circleci/config.yml Lines 354 to 356 in 6f18002
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ |
No, it was set to 1.0.1 before, I asked him to change it to |
Thank you so much @stevedanomodolor for your great work here! This is greatly appreciated! Let me know how your hunt is going and if I can be of help :-) |
Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Future work that may be required in bullet points
For Maintainers: