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

ansible-test - Update sanity test requirements #78324

Closed
wants to merge 4 commits into from

Conversation

samdoran
Copy link
Contributor

SUMMARY

Update pylint to 2.9.6 to get around a false positive for undefined-variable in a decorator.

pylint-dev/pylint#3791

ISSUE TYPE
  • Test Pull Request
COMPONENT NAME

test/lib/ansible_test/_data/requirements/sanity.pylint.in
test/lib/ansible_test/_data/requirements/sanity.pylint.txt

ADDITIONAL INFORMATION
Simple reproducer
import pytest

data = {
    "one": [{"out": "out", "expected": "expected"}],
    "two": [{"out": "out", "expected": "expected"}],
}


@pytest.mark.parametrize(
    ("key", "item", "expected"),
    ((key, case["out"], case["expected"]) for key in data for case in data[key])
)
def some_test(key, item, expected):
    assert key
    assert item
    assert expected
Command output
> ansible-test sanity --docker default --python default example.py

Starting new "ansible-test-controller-I5CwyzNd" container.
Adding "ansible-test-controller-I5CwyzNd" to container database.
Running sanity test "action-plugin-docs"
Running sanity test "ansible-doc"
Running sanity test "changelog"
Running sanity test "compile" on Python 2.7
Running sanity test "compile" on Python 3.5
Running sanity test "compile" on Python 3.6
Running sanity test "compile" on Python 3.7
Running sanity test "compile" on Python 3.8
Running sanity test "compile" on Python 3.9
Running sanity test "compile" on Python 3.10
Running sanity test "compile" on Python 3.11
Running sanity test "empty-init"
Running sanity test "future-import-boilerplate"
Running sanity test "ignores"
Running sanity test "import" on Python 2.7
Running sanity test "import" on Python 3.5
Running sanity test "import" on Python 3.6
Running sanity test "import" on Python 3.7
Running sanity test "import" on Python 3.8
Running sanity test "import" on Python 3.9
Running sanity test "import" on Python 3.10
Running sanity test "import" on Python 3.11
Running sanity test "line-endings"
Running sanity test "metaclass-boilerplate"
Running sanity test "no-assert"
Running sanity test "no-basestring"
Running sanity test "no-dict-iteritems"
Running sanity test "no-dict-iterkeys"
Running sanity test "no-dict-itervalues"
Running sanity test "no-get-exception"
Running sanity test "no-illegal-filenames"
Running sanity test "no-main-display"
Running sanity test "no-smart-quotes"
Running sanity test "no-unicode-literals"
Running sanity test "pep8"
Running sanity test "pslint"
Running sanity test "pylint"
ERROR: Found 1 pylint issue(s) which need to be resolved:
ERROR: example.py:11:75: undefined-variable: Undefined variable 'key'
See documentation for help: https://docs.ansible.com/ansible-core/devel/dev_guide/testing/sanity/pylint.html
Running sanity test "replace-urlopen"
Running sanity test "runtime-metadata"
Running sanity test "shebang"
Running sanity test "shellcheck"
Running sanity test "symlinks"
Running sanity test "use-argspec-type-path"
Running sanity test "use-compat-six"
Running sanity test "validate-modules"
Running sanity test "yamllint"
FATAL: The 1 sanity test(s) listed below (out of 46) failed. See error output above for details.
pylint
FATAL: Command "docker exec ansible-test-controller-I5CwyzNd /usr/bin/env ANSIBLE_TEST_CONTENT_ROOT=/root/ansible_collections/community/general LC_ALL=en_US.UTF-8 /usr/bin/python3.10 /root/ansible/bin/ansible-test sanity example.py --containers '{}' --metadata tests/output/.tmp/metadata-79iptrbd.json --truncate 173 --color yes --host-path tests/output/.tmp/host-63lcc6wn" returned exit status 1.

I've found I can work around this by using different variable names in the comprehension than the function signature.

Update pylint to 2.9.6 to get around a false positive for
undefined-variable in a decorator.

pylint-dev/pylint#3791
@samdoran
Copy link
Contributor Author

I figured this would break some things, but I was hoping it was a small enough change it wouldn't.

@ansibot ansibot added affects_2.14 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. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. 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 21, 2022
@samdoran
Copy link
Contributor Author

Ah, that's another bug in pylint that was fixed already. I'll bump the version gently.

@samdoran
Copy link
Contributor Author

Turning into a bit of a rabbit hole...

This is the minimum version that addressesd the original error I was
seeing that does not fail with other pylint bugs.
@samdoran
Copy link
Contributor Author

samdoran commented Jul 21, 2022

I disabled a few checkers that are new for pylint 2.10. I'm not sure if those should be disabled in the collection.cfg as well.

@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 21, 2022
Only the use-dict-literal is failing, but it’s probably best to disable
the other rules as well so collection authors don’t get any unpleasant
surprises.
@ansibot
Copy link
Contributor

ansibot commented Jul 21, 2022

The test ansible-test sanity --test pylint [explain] failed with 7 errors:

test/lib/ansible_test/_internal/io.py:83:11: unspecified-encoding: Using open without explicitly specifying an encoding
test/lib/ansible_test/_internal/util.py:320:0: superfluous-parens: Unnecessary parens after 'in' keyword
test/lib/ansible_test/_util/target/sanity/compile/compile.py:9:12: redundant-u-string-prefix: The u prefix for strings is no longer necessary in Python >=3.0
test/lib/ansible_test/_util/target/setup/requirements.py:50:12: redundant-u-string-prefix: The u prefix for strings is no longer necessary in Python >=3.0
test/lib/ansible_test/_util/target/setup/requirements.py:260:52: redundant-u-string-prefix: The u prefix for strings is no longer necessary in Python >=3.0
test/lib/ansible_test/_util/target/setup/requirements.py:261:52: redundant-u-string-prefix: The u prefix for strings is no longer necessary in Python >=3.0
test/lib/ansible_test/_util/target/setup/requirements.py:299:11: unspecified-encoding: Using open without explicitly specifying an encoding

click here for bot help

@ansibot ansibot added 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 21, 2022
@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Jul 21, 2022
@mattclay
Copy link
Member

@samdoran I have a local branch where I've started work on upgrading pylint to support Python 3.11. It includes most of the changes you have here, and more, since it's moving to an even newer version of pylint. There's probably not much point in doing this update yet since we'll need to follow it up with another.

@samdoran
Copy link
Contributor Author

@mattclay Sounds good! I'll close this.

@samdoran samdoran closed this Jul 21, 2022
@samdoran samdoran deleted the ci/update-pylint branch July 21, 2022 20:10
@ansible ansible locked and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.14 needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants