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

Set fetch_url() as optionally injectable into aci.py #57071

Open
wants to merge 36 commits into
base: devel
from

Conversation

Projects
None yet
2 participants
@RobW3LGA
Copy link
Contributor

commented May 28, 2019

SUMMARY
  • Allows ACI modules to be testable
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • Modifies all 'fetch_url()' calls in aci.py to 'self.fetch_url()'
ADDITIONAL INFORMATION
  • A mocked fetch_url() allows ACI modules to be properly unit tested
  • This change will also allow ACIModule to become testable
  • The mocked fetch_url() would be injectable by attaching the method to AnsibleModule. When an ACI module is called, it creates an instance of AnsibleModule and sends it into ACIModule. This process remains the same
  • This proposed change does not disrupt current functionality

Line 121 as scoped under init(...)

# Inject fetch_url() if provided
if self.module.fetch_url:
    self.fetch_url = self.module.fetch_url
else:
    self.fetch_url = fetch_url

RobW3LGA added some commits Mar 7, 2019

Updates aci.py with the ability to add ACI objects to any depth
Changes start at line 411 (construct_deep_url() and supporting functions). One minor change to line 633 (the original construct_url()) to provide for testability:  ...join(sorted(self.child_classes)) vs ...join(self.child_classes)

I am also attaching two test files. One characterizing the existing construct_url() and the matching test set for construct_deep_url() to support my efforts and proof of parity
Two PyTest files to support construct_deep_url
These two files provide testing parity, one characterizing the original construct_url() function and the other proofing construct_deep_url(). The ...deep_url.py test file goes five layers deep to provide better validation for the function
Correcting previous upload to incorrect folder
These two files provide testing parity, one characterizing the original construct_url() function and the other proofing construct_deep_url(). The ...deep_url.py test file goes five layers deep to provide better validation for the function
@RobW3LGA

This comment has been minimized.

Copy link
Contributor Author

commented on 13b7af7 Mar 7, 2019

Please cancel this pull request. I have re-uploaded these to the correct folder per Matt Clay (thanks for the help, Matt)

RobW3LGA added some commits Mar 7, 2019

Correcting file names per Matt Clay
@mattclay Thanks again for your continued guidance and patience. Please cancel the previous (incorrect) request
First attempt to comply with suggestions
lib/ansible/module_utils/network/aci/aci.py:517:0: SyntaxWarning: "is not" with a literal. Did you mean "!="?
lib/ansible/module_utils/network/aci/aci.py:534:0: SyntaxWarning: "is not" with a literal. Did you mean "!="?
lib/ansible/module_utils/network/aci/aci.py:558:161: E501 line too long (210 > 160 characters)
First attempt to comply with suggestions
test/units/module_utils/network/aci/test_aci_construct_url.py:1:14: SyntaxError: import pytest
test/units/module_utils/network/aci/test_aci_deep_url.py:1:14: SyntaxError: import pytest
test/units/module_utils/network/aci/test_aci_construct_url.py:0:0: use "\n" for line endings instead of "\r\n"
test/units/module_utils/network/aci/test_aci_deep_url.py:0:0: use "\n" for line endings instead of "\r\n"
Shortened test function names (less descriptive)
Pro Tip: Convert from 'CRLF' to 'LF' in VSCode
It's on the status bar to the right
Added two mocks to support testing
I could not find where to place fakes/mocks, so please let me know if the current location is incorrect
Added last blank line to mock_basic.py
To pass sanity test
Merge pull request #1 from ansible/devel
Re-sync forked repo

RobW3LGA added some commits May 28, 2019

Updates aci.py with the ability to test ACI modules
This update optionally allows for the injection of fetch_url() (normally provided by urls.py) by attaching a mocked fetch_url() to an injectable version of AnsibleModule (normally provided by basic.py). Proposed changes start at line 121.
@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

@RobW3LGA this PR contains the following merge commits:

Please rebase your branch to remove these commits.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

@RobW3LGA This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

commented May 28, 2019

The test ansible-test sanity --test pep8 [explain] failed with 27 errors:

lib/ansible/module_utils/network/aci/aci.py:184:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:185:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:186:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:187:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:332:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:333:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:334:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:335:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:336:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:369:33: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:370:33: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:371:33: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:372:33: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:373:33: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:838:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:839:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:840:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:841:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:974:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:975:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:976:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:977:32: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:1075:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:1076:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:1077:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:1078:36: E128 continuation line under-indented for visual indent
lib/ansible/module_utils/network/aci/aci.py:1079:36: E128 continuation line under-indented for visual indent

click here for bot help

Correcting test failures
Added indents

@ansibot ansibot removed the ci_verified label May 28, 2019

RobW3LGA added some commits May 28, 2019

Retrying...
Last test run cancelled out?
Retrying...
Investigating test run cancellations
Correcting over-indent error
Lines 369-373

@ansibot ansibot added the stale_ci label Jun 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.