Create a framework of functional tests for configuration files #5287

Pierre-Sassoulas · 2021-11-10T08:29:56Z

Type of Changes

	Type
✓	✨ New feature
✓	🔨 Refactoring
✓	📜 Docs

Description

This create a framework for configuration testing. Also migrate three standard unittest to the new framework for testing. Done to help with #4720.

coveralls · 2021-11-10T08:36:30Z

Pull Request Test Coverage Report for Build 1454620578

61 of 61 (100.0%) changed or added relevant lines in 3 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.04%) to 93.402%

Totals
Change from base Build 1453766788:	0.04%
Covered Lines:	13873
Relevant Lines:	14853

💛 - Coveralls

DanielNoord

Great work! I think the functional_append and functional_remove and not the easiest to understand for new contributors, but they probably also shouldn't need to mess with it too much.

Left some comments here and there but in general this seems quite polished!

pylint/testutils/configuration_test.py

tests/config/test_functional_config_loading.py

pylint/testutils/configuration_test.py

See #5287 (comment)

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r746971021

pylint/config/option_manager_mixin.py

Pierre-Sassoulas

Should be good to go review again 😄

DanielNoord

Minor comments here and there.

Should we also test a pylintrc without any extension in this test framework?

pylint/testutils/configuration_test.py

pylint/config/option_manager_mixin.py

tests/config/functional/toml/issue_4746/loaded_plugin_does_not_exists.toml

tests/config/test_functional_config_loading.py

Pierre-Sassoulas · 2021-11-12T11:46:38Z

Should we also test a pylintrc without any extension in this test framework?

Pylintrc is an ini file, I think the file name itself does not matter a lot. (The 'find the conf file' logic is handled elsewhere)

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748064197

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748118048

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748122852

tests/lint/test_pylinter.py

Also migrate three standard unittest to the new framework for testing.

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

See #5287 (comment)

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r746971021

This permits to introduce an example of configuration file with an error.

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748064197

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748118048

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748122852

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748115210

pylint/config/option_manager_mixin.py

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

DanielNoord · 2021-11-13T16:19:42Z

@Pierre-Sassoulas Did you test this with tox? The test break for me locally when using tox. pytest works fine.

Pierre-Sassoulas · 2021-11-13T16:25:22Z

No I'm using only pytest but I thought github action used tox. Let me check.

Detected in pylint-dev#5287 (comment)

Pierre-Sassoulas · 2021-11-14T08:07:05Z

See #5301

Detected in #5287 (comment)

DanielNoord · 2022-03-29T20:29:56Z

pylint/config/option_manager_mixin.py

@@ -271,6 +271,7 @@ def read_config_file(self, config_file=None, verbose=None):

        use_config_file = config_file and os.path.exists(config_file)
        if use_config_file:
+            self.set_current_module(config_file)


@Pierre-Sassoulas Why is this necessary? Does this add anything other than allowing the test framework to function?

Ran into this while working on argparse.

I think it's setting the file that the add_message will display if we add a message later. I.E. [relpath}:1:0: E0013: later.

Pierre-Sassoulas added Configuration Related to configuration Maintenance Discussion or action around maintaining pylint or the dev workflow labels Nov 10, 2021

Pierre-Sassoulas added this to the 2.12.0 milestone Nov 10, 2021

Pierre-Sassoulas requested review from DanielNoord and AWhetter November 10, 2021 08:33

Pierre-Sassoulas force-pushed the add-functional-tests-for-configuration branch from c82225f to bf7f703 Compare November 10, 2021 17:21

DanielNoord requested changes Nov 11, 2021

View reviewed changes

Pierre-Sassoulas force-pushed the add-functional-tests-for-configuration branch from 0117951 to 6b73b8e Compare November 12, 2021 07:22

Pierre-Sassoulas added a commit that referenced this pull request Nov 12, 2021

Simplification of for loop for functional conf test following review

2eaad65

See #5287 (comment)

Pierre-Sassoulas added a commit that referenced this pull request Nov 12, 2021

Better explanation on why we use logging following review

da4c8fe

See #5287 (comment)

Pierre-Sassoulas referenced this pull request Nov 12, 2021

Add a docstring to get_expected_or_default following review

0520fe4

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r746971021

Pierre-Sassoulas force-pushed the add-functional-tests-for-configuration branch from 086766c to 1f615a8 Compare November 12, 2021 08:36

Pierre-Sassoulas commented Nov 12, 2021

View reviewed changes

pylint/config/option_manager_mixin.py Outdated Show resolved Hide resolved

Pierre-Sassoulas commented Nov 12, 2021

View reviewed changes

Pierre-Sassoulas requested a review from DanielNoord November 12, 2021 08:39

DanielNoord reviewed Nov 12, 2021

View reviewed changes

Pierre-Sassoulas referenced this pull request Nov 12, 2021

Add proper typing in configuration test following review

8813782

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748064197

Pierre-Sassoulas referenced this pull request Nov 12, 2021

Remove option that are not required for the test itself

8154ae6

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748118048

Pierre-Sassoulas referenced this pull request Nov 12, 2021

Proper import for python import of CaptureFixture

c166b28

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748122852

DanielNoord reviewed Nov 12, 2021

View reviewed changes

tests/lint/test_pylinter.py Show resolved Hide resolved

Pierre-Sassoulas and others added 8 commits November 12, 2021 18:00

Create a framework of functional tests for configuration files

6626c23

Also migrate three standard unittest to the new framework for testing.

Better handling of removal in functional conf tests

88d08e2

Simplify a list creation in OptionParser()

a450796

Easier understanding of what happen for the expected result

a26e356

Apply suggestions from code review

a8e61d3

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

Update pylint/testutils/configuration_test.py

476ce6b

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

Move test_can_read_toml_rich_types to functional framework

2146e3d

Factorize duplicated code from test_config using the new testutil

0676bfb

Pierre-Sassoulas and others added 14 commits November 12, 2021 18:00

More precise typing for Configuration type when Any is used

e9c89f4

Create a constant for accepted configuration extension

38b7c41

Simplification of for loop for functional conf test following review

58a1d66

See #5287 (comment)

Better explanation on why we use logging following review

2a98bb0

See #5287 (comment)

Add a docstring to get_expected_or_default following review

774c373

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r746971021

Simplification of for loop on dict by using items()

47dae6a

Rename variable that was still named for toml only

388a2db

Add a regression test for #4746

b1f3b41

This permits to introduce an example of configuration file with an error.

Apply suggestions from code review

59e119d

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

Add proper typing in configuration test following review

a08e38e

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748064197

Remove option that are not required for the test itself

7fac09b

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748118048

Proper import for python import of CaptureFixture

4a50fa5

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748122852

Use the existing function for relative path instead of a custom one

dddaf0d

Simplification in OptionManagerMixin following review

10135bb

See https://github.com/PyCQA/pylint/pull/5287\#discussion_r748115210

Pierre-Sassoulas force-pushed the add-functional-tests-for-configuration branch from 3895bd8 to 10135bb Compare November 12, 2021 17:00

DanielNoord approved these changes Nov 12, 2021

View reviewed changes

pylint/config/option_manager_mixin.py Outdated Show resolved Hide resolved

Update pylint/config/option_manager_mixin.py

a4a5a3a

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>

Pierre-Sassoulas merged commit c62738b into main Nov 12, 2021

Pierre-Sassoulas deleted the add-functional-tests-for-configuration branch November 12, 2021 21:19

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this pull request Nov 14, 2021

Fix user specific path in functional conf tests for tox

e847107

Detected in pylint-dev#5287 (comment)

Pierre-Sassoulas mentioned this pull request Nov 14, 2021

Fix user specific path in functional conf tests for tox #5301

Merged

Pierre-Sassoulas added a commit that referenced this pull request Nov 14, 2021

Fix user specific path in functional conf tests for tox (#5301)

e5082d3

Detected in #5287 (comment)

Pierre-Sassoulas mentioned this pull request Nov 14, 2021

Documentation page for configuration file #3181

Closed

cdce8p mentioned this pull request Nov 22, 2021

Primer tests "à la mypy" #5173

Merged

6 tasks

DanielNoord reviewed Mar 29, 2022

View reviewed changes

DanielNoord mentioned this pull request Mar 29, 2022

Move setting of the current module to a better place in config init #6046

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a framework of functional tests for configuration files #5287

Create a framework of functional tests for configuration files #5287

Pierre-Sassoulas commented Nov 10, 2021

coveralls commented Nov 10, 2021 •

edited

Loading

DanielNoord left a comment

Pierre-Sassoulas left a comment

DanielNoord left a comment

Pierre-Sassoulas commented Nov 12, 2021

DanielNoord commented Nov 13, 2021

Pierre-Sassoulas commented Nov 13, 2021

Pierre-Sassoulas commented Nov 14, 2021

DanielNoord Mar 29, 2022

Pierre-Sassoulas Mar 29, 2022

Create a framework of functional tests for configuration files #5287

Create a framework of functional tests for configuration files #5287

Conversation

Pierre-Sassoulas commented Nov 10, 2021

Type of Changes

Description

coveralls commented Nov 10, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1454620578

💛 - Coveralls

DanielNoord left a comment

Choose a reason for hiding this comment

Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

DanielNoord left a comment

Choose a reason for hiding this comment

Pierre-Sassoulas commented Nov 12, 2021

DanielNoord commented Nov 13, 2021

Pierre-Sassoulas commented Nov 13, 2021

Pierre-Sassoulas commented Nov 14, 2021

DanielNoord Mar 29, 2022

Choose a reason for hiding this comment

Pierre-Sassoulas Mar 29, 2022

Choose a reason for hiding this comment

coveralls commented Nov 10, 2021 •

edited

Loading