pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.34k stars 1.14k forks source link

Regression failure when testing with latest version of astroid #10018

Open akamat10 opened 1 month ago

akamat10 commented 1 month ago

Bug description

When testing pylint against latest astroid in repo, there are regression failures. This is tied to an attempt to fix import paths in https://github.com/pylint-dev/astroid/pull/2589 that is needed to support source-roots fallback https://github.com/pylint-dev/pylint/pull/9967 and also clean up the logic.

The following is the output against the latest astroid repo:

=============== 6 failed, 1891 passed, 266 skipped, 5 xfailed in 99.23s (0:01:39) ================

One of the failures is in TestExpandModules.test_expand_modules:

  Full diff:
    {
        '/Users/akhilkamat/Documents/python/pylint/tests/reporters/unittest_json_reporter.py': {
            'basename': 'reporters',
            'basepath': '/Users/akhilkamat/Documents/python/pylint/tests/reporters/__init__.py',
            'isarg': False,
  -         'name': 'reporters.unittest_json_reporter',
  ?                  ----------
  +         'name': 'unittest_json_reporter',
            'path': '/Users/akhilkamat/Documents/python/pylint/tests/reporters/unittest_json_reporter.py',
        },
        '/Users/akhilkamat/Documents/python/pylint/tests/reporters/unittest_reporting.py': {
            'basename': 'reporters',
            'basepath': '/Users/akhilkamat/Documents/python/pylint/tests/reporters/__init__.py',
            'isarg': False,
  -         'name': 'reporters.unittest_reporting',
  ?                  ----------
  +         'name': 'unittest_reporting',
            'path': '/Users/akhilkamat/Documents/python/pylint/tests/reporters/unittest_reporting.py',
        },
    }

The root cause is this line here. This code is making an incorrect assumption that "." is supposed to be current directory and is understood by modutils.modpath_from_file when in fact this api doesn't have any notion of "." being current directory. I tried changing "." to os.getcwd() and this case gets fixed BUT then other tests start breaking. At least a couple of them appear to be false assumptions around what the qualified module names need to be. I will need time to fully understand it.

In the meantime, can I change modutils.modpath_from_file to have a flag (called say prepend_additional_paths) that allows me to introduce this new (what I think is correct behavior) and use this for https://github.com/pylint-dev/pylint/pull/9967 while keeping old behavior if prepend_additional_paths is false? We can slowly analyze the failures, fix the tests and decide if we want to break the backwards compatibility in the future.

Configuration

None

Command used

None

Pylint output

None

Expected behavior

Explained above

Pylint version

astroid 4.0.0.dev0 pylint 4.0.0.dev0

OS / Environment

No response

Additional dependencies

No response

jacobtylerwalls commented 1 month ago

Thanks for looking into it. If the flag is temporary to give us time to evaluate the total effect, +1 from me.

akamat10 commented 1 month ago

Thank you! Ideally, I don't want it to temporary for too long. I will see if I will attempt to fix the tests with the existing change over the weekend. I will report my findings.