Skip to content

Fix ImportedModuleTests #1552

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

Merged
merged 1 commit into from
May 13, 2022

Conversation

jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented May 12, 2022

Description

Type of Changes

Type
βœ“ πŸ› Bug fix

Related Issue

Follow-up to #1399. This needed to subclass TestCase to actually run, and that revealed that clearing the cache before each test method was necessary. cc @tristanlatr EDIT: er, better to use new module names, and avoid clearing the cache altogether, since that's not what's being tested.

@jacobtylerwalls jacobtylerwalls added this to the 2.12.0 milestone May 12, 2022
@jacobtylerwalls jacobtylerwalls added the Maintenance Discussion or action around maintaining astroid or the dev workflow label May 12, 2022
@coveralls
Copy link

coveralls commented May 12, 2022

Pull Request Test Coverage Report for Build 2316558741

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 91.717%

Totals Coverage Status
Change from base Build 2316469071: 0.0%
Covered Lines: 9180
Relevant Lines: 10009

πŸ’› - Coveralls

Copy link
Member Author

@jacobtylerwalls jacobtylerwalls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could conform these to the way Marc did them in #1553, but I think it harms readability.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the switch to pytest style, it's not astroid style for legacy reason but we need to start somewhere if we want good thing πŸ˜„

@jacobtylerwalls jacobtylerwalls merged commit 78f9ece into pylint-dev:main May 13, 2022
@jacobtylerwalls jacobtylerwalls deleted the fix-import-tests branch May 13, 2022 11:53
@@ -6628,38 +6628,60 @@ def test_inference_of_items_on_module_dict() -> None:
builder.file_build(str(DATA_DIR / "module_dict_items_call" / "test.py"), "models")


class ImportedModuleTests(resources.AstroidCacheSetupMixin):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note, you don't need to remove test classes. Even with pytest, they can be helpful to group test cases. I believe the only important things to remember are the classname must begin with Test... and you can use @staticmethod if you don't need self.

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 astroid or the dev workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants