Add regression test for #3651

[jacobtylerwalls]

Type of Changes

	Type
✓	🐛 Bug fix

Description

Closes #3651 with test

[jacobtylerwalls]

Ah, ok, some test isolation issues to look into. It passes in isolation.
Now it's blocked on #4444, which has a WIP patch at #7114, but that patch breaks this test ONLY when run in the context of the full suite. So the next step (eventually) is to find out if that's a real problem or merely a test isolation issue. Marking as draft for now.

[Pierre-Sassoulas]

So the next step (eventually) is to find out if that's a real problem

Our custom context managers could be at fault, I'm thinking in particular of _test_sys_pathand _test_cwd that could be replaced by tmpdir.as_cwd() but I did not find the time yet (some are tricky to replace).

[jacobtylerwalls]

Found the test pollution issue!

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 66bda55

[Pierre-Sassoulas]

That's a really great catch !

[jacobtylerwalls]

This test passes for me locally, but I don't have the capacity to look into why it's failing on CI.

[DanielNoord]

@jacobtylerwalls Test does pass on 3.7 and raises parse-error on other interpreters. Is this using syntax that was changed in 3.8+?

[jacobtylerwalls]

It passes locally for me on all interpreters, so something to investigate with path/cwd vagaries on CI?

[Pierre-Sassoulas]

I opened Pierre-Sassoulas#157 to investigate, I don't have pypy locally so I'm going this with the CI and it's not really practical.

[DanielNoord]

@jacobtylerwalls I did a little investigation and I could get this to fail with pytest -k unittest_lint. The fail is related to x/__init__.py not existing. By changing the test to use linter.check(["x/y"]) the failure goes away but I'm not sure if that breaks the intention of the test. Would that be a suitable fix?

[jacobtylerwalls]

Unfortunately yes, that would break the intention of the test.

Possibly relevant: #3651 (comment)

[DanielNoord]

Hm, but it seems as if the failed test is due to something else? Namespace packages not being listed correctly? If you lint x/y it does work since that is a real package. For directory x with only subdirectory y we don't recognise that it is a linkable package, isn't that expected behaviour?

[jacobtylerwalls]

Ah, you're right!

[DanielNoord]

@jacobtylerwalls So, should the test be changed to close the issue? Or do we need another type of test for this issue?

[jacobtylerwalls]

Yes, I have just pushed an update and reopened the PR. Thanks for walking me through it. (There were a lot of astroid changes recently, and I got distracted based on the author's original expectation that you could lint x.)

[coveralls]

Pull Request Test Coverage Report for Build 2898835605

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 121 unchanged lines in 7 files lost coverage.
• Overall coverage decreased (-0.0009%) to 95.254%

---

Files with Coverage Reduction	New Missed Lines	%
pylint/epylint.py	2	83.33%
pylint/checkers/classes/special_methods_checker.py	8	94.97%
pylint/checkers/format.py	10	96.56%
pylint/checkers/variables.py	14	97.36%
pylint/checkers/refactoring/refactoring_checker.py	16	98.19%
pylint/lint/pylinter.py	23	95.25%
pylint/checkers/classes/class_checker.py	48	94.74%

Totals
Change from base Build 2770034625:	-0.0009%
Covered Lines:	16881
Relevant Lines:	17722

---

💛 - Coveralls