Take column into account in functional tests

[Pierre-Sassoulas]

Description

Surprisingly the column is not taken into account in functional test. This fix that and upgrade the expected result for functional test..

Type of Changes

	Type
✓	✨ New feature

Related Issue

#4003

[coveralls]

Coverage decreased (-0.03%) to 90.671% when pulling d170e17 on Pierre-Sassoulas:take-column-into-account-in-functional-tests into 168bee7 on PyCQA:master.

[Pierre-Sassoulas]

This is more complicated than I thought. I need to find a way to make it work on python 3.8 and 3.9 at least. Apparently the column change a lot between 3.5, 3.6, 3.7 and 3.8.

[hippo91]

@Pierre-Sassoulas at first sight it is a nice improvment! 👍
Let me know when you want me to have another look!

[Pierre-Sassoulas]

@hippo91 I think this is ready to review. I wanted to wait for #3965 to be ready but the fix for python 3.5 were not that hard to make. (I don't understand how the tests are launched in python 3.5 as some typing prevent to launch them on my side, but anyway... I digress).

The real column is not taken into account until python 3.8 because the AST changed at this point sp the value do not make as much sense before 3.8.

[Pierre-Sassoulas]

@hippo91 I think we should merge this soon because it will cause a conflict on each functional test we need to rebase it on, so the less time in review the better.

[hippo91]

@Pierre-Sassoulas i already started the review. I ll try to finish it today.

[hippo91]

@Pierre-Sassoulas great work! 🎆 👍
I left just a few comments but nothing that prevent a merge in master!

[hippo91]

I do not find the call place of this method...

[Pierre-Sassoulas]

l.72 and l.106 in tests/test_func.py and l. 167 in pylint/testutils/lint_module_test.py :)

[Pierre-Sassoulas]

Actually the instance is initialized here:

@pytest.mark.parametrize("test_file", TESTS, ids=TESTS_NAMES)
def test_functional(test_file, recwarn):
    if UPDATE_FILE.exists():
        lint_test = LintModuleOutputUpdate(test_file)
    else:
        lint_test = testutils.LintModuleTest(test_file)

If we're using the update we instantiate a LintModuleOutputUpdate that will be using this function later.

[hippo91]

@Pierre-Sassoulas i'm sorry but i don't get it.
It seems to me that in tests/test_func.py l.72 and l.106 it calls the method defined in the same module at line 93.
The call at l167 in lint_module_test.py seems to be related to the method defined at line 140 of the same module.
I'm i wrong?

[Pierre-Sassoulas]

Regarding the call line 167 in lint_module_test.py It the update file exists we instantiate a child class LintModuleOutputUpdate of the mother class LintModuleTest (code in comment above, from test_functional.py). Then when reaching _get_expected l167 what is called is not the one defined l140 in LintModuleTest the but the one we're talking about overidden in the child class. Basically it comes done to this snippet with Mother=LintModuleTest and Child=LintModuleOutputUpdate:

class Mother:
    
    def __init__(self):
        print("Get expected is : {}".format(self._get_expected()))
        
    def _get_expected(self):
        return "Mother implementation."

class Child(Mother):
    
        def _get_expected(self):
            return "Child implementation."


Mother() # Get expected is : Mother implementation.                                                                                                                               
Child() # Get expected is : Child implementation.   

[hippo91]

Ok! Thanks @Pierre-Sassoulas