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

Fix collection loader and add unit tests. #58701

Merged
merged 8 commits into from
Jul 10, 2019

Conversation

mattclay
Copy link
Member

@mattclay mattclay commented Jul 4, 2019

SUMMARY

Fix collection loader and add unit tests.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

collection loader

ADDITIONAL INFORMATION

Use compile before eval in collection loader.

This fixes two issues:

  1. File names are available when tracing execution, such as with code coverage.
  2. Future statements are not inherited from the collection loader.

Add unit tests for collection loading.

These tests verify several things:

  1. That unit tests can import code from collections when the collection loader is installed.
  2. That tracing reports the correct file and line numbers (to support code coverage).
  3. That collection code does not inherit __future__ statements from the collection loader.

This fixes two issues:

1. File names are available when tracing execution, such as with code coverage.
2. Future statements are not inherited from the collection loader.
These tests verify several things:

1. That unit tests can import code from collections when the collection loader is installed.
2. That tracing reports the correct file and line numbers (to support code coverage).
3. That collection code does not inherit __future__ statements from the collection loader.
@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. new_plugin This PR includes a new plugin. support:core This issue/PR relates to code supported by the Ansible Engineering Team. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jul 4, 2019
Since the collection loader is installed simply by importing ansible.plugins.loader,
we may already have a collection loader installed when the test runs. This occurs if
any other tests are collected which use that import during collection. Until that code
is moved into an initialization function to avoid loading during import, the unit tests
will need to replace any existing collection loaders so that they reflect the desired
configuration.
@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Jul 4, 2019
@mattclay mattclay requested a review from nitzmahone July 4, 2019 07:25
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 4, 2019
This is a requirement of PEP 302.

It will prevent recursion errors when importing the current module or using a relative import.
This allows using relative imports in collections.
The relative-beyond-top-level rule in pylint may not be appropriate for collections.
However, until that rule is disabled for collections this will keep tests passing.
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. core_review In order to be merged, this PR must follow the core review workflow. and removed core_review In order to be merged, this PR must follow the core review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jul 8, 2019
@mattclay mattclay merged commit 9e67953 into ansible:devel Jul 10, 2019
@mattclay mattclay deleted the collection-loader branch July 10, 2019 00:31
@ansible ansible locked and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. new_plugin This PR includes a new plugin. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants