-
Notifications
You must be signed in to change notification settings - Fork 276
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
Don't use $HOME on most tests (InternalFixture) #971
Conversation
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Codecov Report
@@ Coverage Diff @@
## ign-gazebo3 #971 +/- ##
===============================================
- Coverage 77.88% 77.86% -0.03%
===============================================
Files 221 221
Lines 12681 12683 +2
===============================================
- Hits 9877 9876 -1
- Misses 2804 2807 +3
Continue to review full report at Codecov.
|
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 know there are other tests still loading ~/.ignition/gazebo/server.config
Should we create an issue with all the tests that are still loading ~/.ignition/gazebo/server.config
so that we can keep track of what needs to be updated?
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Ok, I went ahead and fixed almost all of them (if not all): 886b788 |
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 still need to do a deeper review pass, but before I do, there are a few higher-level questions that I have.
There are also a few codecheck issues:
|
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
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.
LGTM, just one question for clarity/understanding below. Maybe we can get one more set of eyes on this to make sure we aren't introducing any new test failures.
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.
LGTM
Signed-off-by: Louise Poubel louise@openrobotics.org
🦟 Bug fix
Summary
Tests shouldn't be relying on the user's home folder. This makes sure that most tests use the
server.config
from a custom home path.I think I caught all tests that are loading
~/.ignition/gazebo/server.config
. This PR also fixes creating the config file in general, because now we make sure that the directories are created too.It also backports
EnvTestFixture
from #594, with some updates to make sure each test can keep its name, as well as supporting various test types.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge
🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸