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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pollution test cases #1559

Closed
cdce8p opened this issue May 13, 2022 · 2 comments 路 Fixed by #1563
Closed

Pollution test cases #1559

cdce8p opened this issue May 13, 2022 · 2 comments 路 Fixed by #1563
Assignees
Labels
Bug 馃 High effort 馃弸 Difficult solution or problem to solve Maintenance Discussion or action around maintaining astroid or the dev workflow
Milestone

Comments

@cdce8p
Copy link
Member

cdce8p commented May 13, 2022

While debugging #1551, I used dectect-test-pollution to isolate the test case.

The tool does have a fuzzy mode which can find other pollution test cases. Unfortunately, it didn't even make it past round 1 馃槙
It seems like our test suite doesn't clean up properly and is somewhat reliant on a global state. For now it's fine to continue as is though, at some point, these issues might come back to haunt us.

Install

pip install detect-test-pollution

Run with (Python 3.10)

detect-test-pollution --tests tests/ --fuzz --runs 10

If someone would like to take a look, that would be greatly appreciated 馃檹馃徎

/CC: @jacobtylerwalls and @DanielNoord I saw you two worked on caching recently, maybe this is something you could take a look at?

/CC: @Pierre-Sassoulas

--

The first case I saw

Failing test: tests/unittest_brain.py::TestIsinstanceInference::test_isinstance_int_true
Pollution test: tests/unittest_manager.py::ClearCacheTest::test_brain_plugins_reloaded_after_clearing_cache

pytest \
    tests/unittest_manager.py::ClearCacheTest::test_brain_plugins_reloaded_after_clearing_cache \
    tests/unittest_brain.py::TestIsinstanceInference::test_isinstance_int_true 
@cdce8p cdce8p added High effort 馃弸 Difficult solution or problem to solve Maintenance Discussion or action around maintaining astroid or the dev workflow labels May 13, 2022
@Pierre-Sassoulas
Copy link
Member

It seems like our test suite doesn't clean up properly and is somewhat reliant on a global state.

That would be the astroid cache, I guess ? 馃槃 We can clean it up now but recently it was a mutable default value of a function. I don't think we added the cache clean function for each tests nor should we do it imo.

@jacobtylerwalls jacobtylerwalls self-assigned this May 14, 2022
@jacobtylerwalls
Copy link
Member

I found the cause. #1460 introduced a global state that can be wrong if the cache is cleared (and the builtins module is rebuilt). I have a tiny diff to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 馃 High effort 馃弸 Difficult solution or problem to solve Maintenance Discussion or action around maintaining astroid or the dev workflow
Projects
None yet
3 participants