Skip to content

Remove the pytest.ini workaround for distros after Foxy.#497

Merged
clalancette merged 2 commits intomasterfrom
clalancette/remove-pytest-ini-workaround
Aug 4, 2020
Merged

Remove the pytest.ini workaround for distros after Foxy.#497
clalancette merged 2 commits intomasterfrom
clalancette/remove-pytest-ini-workaround

Conversation

@clalancette
Copy link
Copy Markdown
Contributor

After Foxy was released, we added pytest.ini to all of the
packages that required it. This was so that local tests behaved
the same as CI. In Foxy and prior, we still need the workaround
so CI will succeed.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

Still in draft since I need to run tests on it. @j-rivero FYI, this relates to (but does not completely remove the need for) #471. This also relates to ros2/ros2#951

After Foxy was released, we added pytest.ini to all of the
packages that required it.  This was so that local tests behaved
the same as CI.  In Foxy and prior, we still need the workaround
so CI will succeed.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette
Copy link
Copy Markdown
Contributor Author

clalancette commented Jul 23, 2020

CI with distro set to Rolling: Build Status
CI with distro set to Foxy: Build Status

Both are expected to succeed, but the Foxy one should have some extra output at the beginning of the test section that says:

xunit2 patch applied to ...

While the rolling one should not.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@j-rivero
Copy link
Copy Markdown

Still in draft since I need to run tests on it. @j-rivero FYI, this relates to (but does not completely remove the need for) #471. This also relates to ros2/ros2#951

+1 although I keep the warning I wrote here #471 (comment)

@clalancette
Copy link
Copy Markdown
Contributor Author

clalancette commented Jul 23, 2020

Rolling CI failed because we are still on a version before my fix: https://gitlab.com/micro-ROS/ros_tracing/ros2_tracing/-/merge_requests/185 . I'll talk to the ros2_tracing people and see if we can get an update.

https://gitlab.com/micro-ROS/ros_tracing/ros2_tracing/-/issues/98

@clalancette
Copy link
Copy Markdown
Contributor Author

ros2_tracing is now on master, so we can try again. Here's another CI run:

CI with distro set to Rolling: Build Status
CI with distro set to Foxy: Build Status

@clalancette clalancette marked this pull request as ready for review August 4, 2020 13:55
@clalancette
Copy link
Copy Markdown
Contributor Author

+1 although I keep the warning I wrote here #471 (comment)

Right, it is definitely still the case that if any package is added that doesn't have the proper pytest.ini, it will cause the build to fail. While that isn't exactly nice, the good news is that it is very noisy and it should be obvious why it failed. So I still think we should go forward with this.

I'm going to go ahead and do a full CI run on all platforms just to make sure there aren't any surprises with Rolling. After that, I'll probably go ahead and merge unless @j-rivero has any other concerns with this PR.

@clalancette
Copy link
Copy Markdown
Contributor Author

Full CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

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.

3 participants