Conversation
|
This overlaps with #420. |
Yep, pointed out in the initial description. Only the first one does. |
launch_testing/pytest.ini
Outdated
| testpaths = test | ||
| # Add arguments for launch tests | ||
| addopts = --launch-args dut_arg:=test | ||
| junit_family=xunit1 |
There was a problem hiding this comment.
This is not compatible with ci.ros2.org: https://github.com/ros2/ci/blob/a7c41fbce3f1acee2a98a0e2865a8c9a0fb0db8a/ros2_batch_job/__main__.py#L381
There was a problem hiding this comment.
Ah, thanks for the feeback. Will set to xunit2.
There was a problem hiding this comment.
CI already sets the flag automatically. So why do you want to set it explicitly (which would be the only package which does that)?
There was a problem hiding this comment.
Because otherwise there's a warning from CI:
Warning: The 'junit_family' default value will change to 'xunit2' in pytest 6.0.
Add 'junit_family=xunit1' to your pytest.ini file to keep the current format in future versions of pytest and silence this warning.
I think this needs a custom pytest.ini to add options, but I really don't know. @ivanpauno @hidmic any thoughts here?
There was a problem hiding this comment.
I think that if the warning is silenced by explicitly specifying the junit_familiy here and CI passes, it's ok this approach (besides that CI already sets the junit family).
There was a problem hiding this comment.
Because otherwise there's a warning from CI:
Can you please reference the build where you get this warning. On ci.ros2.org this shouldn't happen because of ros2/ci#406.
There was a problem hiding this comment.
I think that if the warning is silenced by explicitly specifying the
junit_familiyhere and CI passes, it's ok this approach (besides that CI already sets the junit family).
The question is why it is necessary since the CI logic tries to workaround this already. And if it is necessary I would assume it is the same for all packages and then should also be addressed for all.
There was a problem hiding this comment.
Oh, right. This doesn't happen in CI because of the override. I only saw it locally, which is why we need it.
There was a problem hiding this comment.
I only saw it locally, which is why we need it.
Then please apply it in a separate pull request to all packages.
There was a problem hiding this comment.
Then please apply it in a separate pull request to all packages.
I really don't see why. This improves it for this package; adding it to other packages to fix it there is orthogonal.
d6224a0 to
99dd781
Compare
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
This avoids a warning like: Warning: The 'junit_family' default value will change to 'xunit2' in pytest 6.0. Add 'junit_family=xunit1' to your pytest.ini file to keep the current format in future versions of pytest and silence this warning. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
2b3213d to
f3d5e86
Compare
|
CI:
|
|
All right, green CI and approval on both this and ros2/launch_ros#150. I'm going to merge these; we can follow up with more pytest warning fixes in a follow-up. |
…391) Switch to from_parent (partial #421) avoid deprecation warning, use from_parent (#402) (#459) * stop using constructors deprecated in pytest 5.4 (#391) Signed-off-by: Dan Rose <dan@digilabs.io> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * Switch to from_parent to remove deprecation warning. Signed-off-by: Chris Lalancette <clalancette@openrobotics.org> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> * avoid deprecation warning, use from_parent (#402) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> Co-authored-by: Dan Rose <rotu@users.noreply.github.com> Co-authored-by: Chris Lalancette <clalancette@openrobotics.org> Co-authored-by: Dirk Thomas <dirk-thomas@users.noreply.github.com>
This PR fixes a bunch of warnings in launch.
(this is fixed in several tests)
3. pytest on Bionic complains that:
With all of these fixes in place, I get green CI again.