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

Re-enable tests of bash completion functions for gz #481

Merged
merged 3 commits into from
Feb 13, 2024

Conversation

mjcarroll
Copy link
Contributor

Follow-up from #478

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
@mjcarroll mjcarroll self-assigned this Feb 12, 2024
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Feb 12, 2024
@mjcarroll mjcarroll mentioned this pull request Feb 12, 2024
Copy link

codecov bot commented Feb 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (eac2e69) 87.69% compared to head (014e40e) 87.69%.
Report is 2 commits behind head on gz-transport13.

Additional details and impacted files
@@               Coverage Diff               @@
##           gz-transport13     #481   +/-   ##
===============================================
  Coverage           87.69%   87.69%           
===============================================
  Files                  59       59           
  Lines                5704     5704           
===============================================
  Hits                 5002     5002           
  Misses                702      702           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@caguero caguero left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

homebrew looks unhappy

@mjcarroll
Copy link
Contributor Author

homebrew looks unhappy

I'm not sure these tests worked on homebrew before, I will limit them to just Linux.

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
@mjcarroll
Copy link
Contributor Author

mjcarroll commented Feb 12, 2024

Trying something slightly different and starting with /usr/bin/bash and falling back to /bin/bash if that isn't available. The subprocess class doesn't do anything with the PATH environment variable, you have to have the full and complete path to the binary in question.

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
@mjcarroll
Copy link
Contributor Author

@scpeters did a brief inspection for me on macOS and things didn't seem to work, likely because of some security mechanism of the operating system. Since the value of this test is mainly in checking the flags for consistency and keeping them in sync between the completion scripts and the executables, running cross-platform isn't really necessary here.

@mjcarroll mjcarroll merged commit cfb80d2 into gz-transport13 Feb 13, 2024
10 checks passed
@mjcarroll mjcarroll deleted the mjcarroll/restore_gz_tests branch February 13, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants