Refactor functional testutil in order to normalize later #5418

Pierre-Sassoulas · 2021-11-28T15:52:39Z

Type of Changes

	Type
✓	🔨 Refactoring

Description

The functional tests grow all the time and organizing them properly will become a problem soon. See possible follow up here: #5396 (comment)

coveralls · 2021-11-28T16:01:45Z

Pull Request Test Coverage Report for Build 1515867452

99 of 102 (97.06%) changed or added relevant lines in 6 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.1%) to 93.642%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pylint/testutils/functional/find_functional_tests.py	14	15	93.33%
pylint/testutils/functional/test_file.py	53	55	96.36%

Totals
Change from base Build 1513600876:	0.1%
Covered Lines:	14065
Relevant Lines:	15020

💛 - Coveralls

DanielNoord

I think we need to add some additional tests for the DeprecationWarning's to make sure this isn't breaking. We have been doing that for other DeprecationWarning's as well.

Think this is a good move to make!

pylint/testutils/functional/find_functional_tests.py

pylint/testutils/functional/lint_module_output_update.py

pylint/testutils/functional/test_file.py

pylint/testutils/functional_test_file.py

tests/test_functional.py

DanielNoord · 2021-11-28T18:36:05Z

tests/testutils/test_lint_module_output_update.py

+
+import pytest
+
+from pylint.testutils import FunctionalTestFile


Shall we add an test to see if the deprecation warning triggers correctly when doing this import?
I think it might be better to add this in a with pytest.warns guard anyway as we will otherwise always trigger the warning ourselves.

I'd like to do that but I don't know what is triggering the deprecation warning, it's supposed to not ever be imported.

What happens if somebody uses the current way to import it? Doesn't that trigger the deprecation?

Well there's a warning when we launch the test at the beginning not when we re-import later in the test:

with pytest.warns(DeprecationWarning) as records: from pylint.testutils.functional_test_file import FunctionalTestFile

Result in :

with pytest.warns(DeprecationWarning) as records: > from pylint.testutils.functional_test_file import FunctionalTestFile E Failed: DID NOT WARN. No warnings of type (<class 'DeprecationWarning'>,) was emitted. The list of emitted warnings is: []. tests/testutils/test_functional_test_file.py:5: Failed ============================================================================================ warnings summary ============================================================================================= pylint/testutils/functional_test_file.py:18 /home/pierre/pylint/pylint/testutils/functional_test_file.py:18: DeprecationWarning: 'pylint.testutils.functional_test_file' will be accessible from the 'pylint.testutils.functional' namespace in pylint 3.0.

Notice how the deprecation Warning is taunting us at the bottom 😄

Somehow there must be an import from this file that I can't seem to find.

tests/testutils/test_lint_module_output_update.py

ChangeLog

pylint/testutils/__init__.py

pylint/testutils/functional/find_functional_tests.py

pylint/testutils/functional/lint_module_output_update.py

pylint/testutils/functional/test_file.py

pylint/testutils/lint_module_test.py

Pierre-Sassoulas · 2021-11-28T22:32:24Z

I'm inclined to revert the typing here and keep it for another MR as the following error appear:

pylint/testutils/lint_module_test.py:60: error: Unsupported operand types for >= ("_version_info" and "List[str]")  [operator]
pylint/testutils/lint_module_test.py:60: note: Right operand is of type "Union[Tuple[int, ...], List[str]]"
pylint/testutils/lint_module_test.py:71: error: Argument 1 to "__import__" has incompatible type "Union[int, str]"; expected "str"  [arg-type]
pylint/testutils/lint_module_test.py:75: error: Argument 1 to "join" of "str" has incompatible type "List[Union[int, str]]"; expected "Iterable[str]"  [arg-type]
pylint/testutils/lint_module_test.py:78: error: Item "Tuple[int, ...]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:78: error: Item "List[str]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:84: error: Item "Tuple[int, ...]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:84: error: Item "List[str]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:93: error: Unsupported operand types for < ("_version_info" and "List[str]")  [operator]
pylint/testutils/lint_module_test.py:93: note: Right operand is of type "Union[Tuple[int, ...], List[str]]"
pylint/testutils/lint_module_test.py:94: error: Unsupported operand types for > ("_version_info" and "List[str]")  [operator]
pylint/testutils/lint_module_test.py:94: note: Right operand is of type "Union[Tuple[int, ...], List[str]]"

options in FunctionalTestFile are a dict and the refactor look like enough work for another MR.

DanielNoord · 2021-11-28T22:39:44Z

I'm inclined to revert the typing here and keep it for another MR as the following error appear:

pylint/testutils/lint_module_test.py:60: error: Unsupported operand types for >= ("_version_info" and "List[str]")  [operator]
pylint/testutils/lint_module_test.py:60: note: Right operand is of type "Union[Tuple[int, ...], List[str]]"
pylint/testutils/lint_module_test.py:71: error: Argument 1 to "__import__" has incompatible type "Union[int, str]"; expected "str"  [arg-type]
pylint/testutils/lint_module_test.py:75: error: Argument 1 to "join" of "str" has incompatible type "List[Union[int, str]]"; expected "Iterable[str]"  [arg-type]
pylint/testutils/lint_module_test.py:78: error: Item "Tuple[int, ...]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:78: error: Item "List[str]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:84: error: Item "Tuple[int, ...]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:84: error: Item "List[str]" of "Union[Tuple[int, ...], List[str]]" has no attribute "split"  [union-attr]
pylint/testutils/lint_module_test.py:93: error: Unsupported operand types for < ("_version_info" and "List[str]")  [operator]
pylint/testutils/lint_module_test.py:93: note: Right operand is of type "Union[Tuple[int, ...], List[str]]"
pylint/testutils/lint_module_test.py:94: error: Unsupported operand types for > ("_version_info" and "List[str]")  [operator]
pylint/testutils/lint_module_test.py:94: note: Right operand is of type "Union[Tuple[int, ...], List[str]]"

options in FunctionalTestFile are a dict and the refactor look like enough work for another MR.

Could you try to see if this works?

if sys.version_info >= (3, 8):
    from typing import TypedDict
else:
    from typing_extensions import TypedDict

class TestFileOptions(TypedDict):
    min_pyver: Tuple[int, ...]
    max_pyver: Tuple[int, ...]
    min_pyver_end_position: Tuple[int, ...]
    requires: List[str]
    except_implementations: List[str]
    exclude_platforms: List[str]

...

self.options: TestFileOptions = {

I think that should do it? If not reverting might indeed be best 😢

Pierre-Sassoulas · 2021-11-29T10:33:39Z

Could you try to see if this works?

Thanks, done in df20ded. It's too big for this MR but I started to cherry-pick on main and there's conflicts... so let's merge everything here.

DanielNoord

Looks good! We actually caught some "bugs" due to the typing 😄

pylint/testutils/functional/lint_module_output_update.py

tests/functional/f/fixme.rc

pylint/testutils/functional/test_file.py

DanielNoord · 2021-11-29T10:33:00Z

pylint/testutils/functional/test_file.py

+            conv = self._CONVERTERS.get(name, lambda v: v)
+
+            assert (
+                name in POSSIBLE_TEST_OPTIONS


TestFileOptions.__annotations__ would also work, but I'm not sure if you're supposed to access it..

I don't like typed dict, it's only marginally better than a dict. Might refactor that class later.

pylint/testutils/functional/test_file.py

ChangeLog

And use Path instead of os

Better typing for test options and fix existing issue

CarliJoy · 2021-11-29T18:33:32Z

Why is this MR blocked?

Pierre-Sassoulas · 2021-11-29T18:50:45Z

Why is this MR blocked?

Waiting for 2.12.2 to be released : #5418 (comment)

Pierre-Sassoulas added Enhancement ✨ Improvement to a component Maintenance Discussion or action around maintaining pylint or the dev workflow labels Nov 28, 2021

Pierre-Sassoulas added this to the 2.12.2 milestone Nov 28, 2021

Pierre-Sassoulas force-pushed the refactor-functional-testutil-for-normalize-later branch from 51fc083 to 9b8ea92 Compare November 28, 2021 15:55

Pierre-Sassoulas mentioned this pull request Nov 28, 2021

fix(4716): fix false positive unnecessary_dict_index_lookup emitted when del is used #5344

Merged

DanielNoord requested changes Nov 28, 2021

View reviewed changes

Pierre-Sassoulas mentioned this pull request Nov 28, 2021

add a checker for misleading unicode #5311

Merged

DanielNoord reviewed Nov 29, 2021

View reviewed changes

DanielNoord approved these changes Nov 29, 2021

View reviewed changes

ChangeLog Show resolved Hide resolved

Pierre-Sassoulas added 10 commits November 29, 2021 13:14

Add a path argument to get_tests for functional tests

ef9af37

Rename get_tests to get_functional_test_files_from_directory

2b02899

And use Path instead of os

Move the filter for isort 5 out of the generic function

e1a123e

Remove a constant that was never used anywhere

360ed42

Deprecate some file in pylint/testutils and update changelog

82918aa

Move the function go retrieve test file to pylint.testutil.functional

c83c567

Move LintModuleOutputUpdate to pylint.testutil.functional

5e78a18

Add unit tests for LintModuleOutputUpdate

3799931

Add a warning in functional test update for bad python versions

8cbbf48

Add typing to LintModuleOutputUpdate._check_output_text

a2a66a6

Better typing for test options and fix existing issue

Pierre-Sassoulas force-pushed the refactor-functional-testutil-for-normalize-later branch from 8135de0 to a2a66a6 Compare November 29, 2021 12:15

Pierre-Sassoulas modified the milestones: 2.12.2, 2.13.0 Nov 29, 2021

Pierre-Sassoulas added the Blocked 🚧 Blocked by a particular issue label Nov 29, 2021

Pierre-Sassoulas removed the Blocked 🚧 Blocked by a particular issue label Dec 3, 2021

Pierre-Sassoulas merged commit 5bb0095 into pylint-dev:main Dec 3, 2021

Pierre-Sassoulas deleted the refactor-functional-testutil-for-normalize-later branch December 3, 2021 17:38

DanielNoord mentioned this pull request Dec 13, 2021

Unify validation of functional test option files #5510

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor functional testutil in order to normalize later #5418

Refactor functional testutil in order to normalize later #5418

Pierre-Sassoulas commented Nov 28, 2021

coveralls commented Nov 28, 2021 •

edited

Loading

DanielNoord left a comment

DanielNoord Nov 28, 2021

Pierre-Sassoulas Nov 28, 2021

DanielNoord Nov 28, 2021

Pierre-Sassoulas Nov 28, 2021

Pierre-Sassoulas commented Nov 28, 2021

DanielNoord commented Nov 28, 2021 •

edited

Loading

Pierre-Sassoulas commented Nov 29, 2021

DanielNoord left a comment

DanielNoord Nov 29, 2021

Pierre-Sassoulas Nov 29, 2021

CarliJoy commented Nov 29, 2021

Pierre-Sassoulas commented Nov 29, 2021


		import pytest

		from pylint.testutils import FunctionalTestFile

Refactor functional testutil in order to normalize later #5418

Refactor functional testutil in order to normalize later #5418

Conversation

Pierre-Sassoulas commented Nov 28, 2021

Type of Changes

Description

coveralls commented Nov 28, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1515867452

💛 - Coveralls

DanielNoord left a comment

Choose a reason for hiding this comment

DanielNoord Nov 28, 2021

Choose a reason for hiding this comment

Pierre-Sassoulas Nov 28, 2021

Choose a reason for hiding this comment

DanielNoord Nov 28, 2021

Choose a reason for hiding this comment

Pierre-Sassoulas Nov 28, 2021

Choose a reason for hiding this comment

Pierre-Sassoulas commented Nov 28, 2021

DanielNoord commented Nov 28, 2021 • edited Loading

Pierre-Sassoulas commented Nov 29, 2021

DanielNoord left a comment

Choose a reason for hiding this comment

DanielNoord Nov 29, 2021

Choose a reason for hiding this comment

Pierre-Sassoulas Nov 29, 2021

Choose a reason for hiding this comment

CarliJoy commented Nov 29, 2021

Pierre-Sassoulas commented Nov 29, 2021

coveralls commented Nov 28, 2021 •

edited

Loading

DanielNoord commented Nov 28, 2021 •

edited

Loading