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

Add methods on Mock class for Python 3.5 compatibility #237

Merged
merged 3 commits into from
Sep 13, 2018

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Sep 13, 2018

This fixes some test failures on xenial caused by #222. The tests used methods on unittest.mock.Mock that were added in python 3.6. This PR ~~~adds a subclass with those methods defined~~~ monkey patches those methods onto the mock class if they are not present.

Xenial CI just rclpy
Build Status

CI bionic just rclpy
Build Status

Will run full CI post review

Connects to ros2/build_farmer#147

@sloretz sloretz added the bug Something isn't working label Sep 13, 2018
@sloretz sloretz self-assigned this Sep 13, 2018
@sloretz sloretz requested a review from dhood September 13, 2018 18:29
@sloretz sloretz added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Sep 13, 2018
@sloretz
Copy link
Contributor Author

sloretz commented Sep 13, 2018

Looks like CI is quick, CI testing just rclpy

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

@dirk-thomas
Copy link
Member

You don't need a subclass you could just add the methods to the existing class.

@sloretz
Copy link
Contributor Author

sloretz commented Sep 13, 2018

@dirk-thomas monkey patch instead of subclass in acc2065

@@ -23,6 +23,8 @@
from rclpy.duration import Duration
from rclpy.time import Time

from .mock_compat import * # noqa
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can't do import .mock_compat becase PEP 328 forbids it, and can't do import mock_compat because pytest doesn't put the test directory into sys.path.

Copy link
Member

Choose a reason for hiding this comment

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

Only suppress the specific category rather than a blank noqa.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

6d8381d gets rid of the * so flake8 can detected undefined names and ignores the specific violation of variable _ being unused.

@sloretz sloretz merged commit 1538e72 into master Sep 13, 2018
@sloretz sloretz deleted the mock_python35_compatibility branch September 13, 2018 23:46
@sloretz sloretz removed the in review Waiting for review (Kanban column) label Sep 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants