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

Regression failure when testing with latest version of astroid #10018

Open
akamat10 opened this issue Oct 13, 2024 · 2 comments
Open

Regression failure when testing with latest version of astroid #10018

akamat10 opened this issue Oct 13, 2024 · 2 comments
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@akamat10
Copy link
Contributor

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 pylint-dev/astroid#2589 that is needed to support source-roots fallback #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 #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

@akamat10 akamat10 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Oct 13, 2024
@jacobtylerwalls
Copy link
Member

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

@jacobtylerwalls jacobtylerwalls added Needs PR This issue is accepted, sufficiently specified and now needs an implementation Maintenance Discussion or action around maintaining pylint or the dev workflow and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Oct 13, 2024
@jacobtylerwalls jacobtylerwalls added this to the 4.0.0 milestone Oct 13, 2024
@akamat10
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Discussion or action around maintaining pylint or the dev workflow Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

2 participants