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

fixed mock.called_once bugs #122

Closed
wants to merge 2 commits into from
Closed

fixed mock.called_once bugs #122

wants to merge 2 commits into from

Conversation

StephanHasler
Copy link
Contributor

@StephanHasler StephanHasler commented Jan 17, 2022

Proposed change(s)

In the test code several 'assert_called_once' have been replaced by 'assert_called_once()'.
Doing so revealed another bug. Following change was necessary

mock_sys_modules.assert_called_once()  # bug: sys.modules["foo"] does not call the mock
mock_sys_modules.__getitem__.assert_called_once() # correct: it calls the mocks's __getitem__ instead

Useful links (GitHub issues, JIRA tickets, forum threads, etc.)

This fixes #121

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe)

Testing and Verification

The unit tests have been executed like:

ROS-TCP-Endpoint/test>python -m pytest test*.py

All 38 tests passed.

Please describe the tests that you ran to verify your changes. Please also provide instructions, ROS packages, and Unity project files as appropriate so we can reproduce the test environment.

Test Configuration:

  • ROS Ubuntu 20.04, ROS Noetic

Checklist

  • Ensured this PR is up-to-date with the dev branch
  • Created this PR to target the dev branch
  • Followed the style guidelines as described in the Contribution Guidelines
  • Added tests that prove my fix is effective or that my feature works
  • Updated the Changelog and described changes in the Unreleased section
  • Updated the documentation as appropriate

Other comments

@unity-cla-assistant
Copy link

unity-cla-assistant commented Jan 17, 2022

CLA assistant check
All committers have signed the CLA.

@StephanHasler StephanHasler closed this by deleting the head repository Apr 3, 2024
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.

2 participants