Changing message parsing in BaseLinter so it respects python.linting.maxNumberOfProblems

[saponas]

Fixes #2198

• Title summarizes what is changing
• Includes a news entry file (remember to thank yourself!)
• Unit tests & code coverage are not adversely affected (within reason)
• Works on all actively maintained versions of Python (e.g. Python 2.7 & the latest Python 3 release)
• Works on Windows 10, macOS, and Linux (e.g. considered file system case-sensitivity)
  Only tested Win10
• Dependencies are pinned (e.g. "1.2.3", not "^1.2.3")
• package-lock.json has been regenerated if dependencies have changed

[brettcannon]

Any chance you feel up to writing a unit test to make sure we don't break this in the future? 😉

[brettcannon]

Could this be simplified?:

for (let line of outputLines) {
    try {
        const msg = this.parseLine(line, regEx);
        if (msg) {
            messages.push(msg);
            if (messages.length >= this.pythonSettings.linting.maxNumberOfProblems) {
                break;
            }
        }
    } catch ...
}
return messages;

[brettcannon]

@saponas no pressure on our part, but we are planning to code freeze for the August release next week, so if you could address my comment before then we can squeeze this in, else it will slip to the September release.

[saponas]

Thanks @brettcannon. I addressed them in a branch, but haven't merged them into this PR yet because I'm having trouble getting all the unit tests to run. (i.e., I'm having trouble testing my unit test :-). Any tips on how to get all the unit tests running correctly other than following the https://github.com/Microsoft/vscode-python/blob/master/CONTRIBUTING.md

[brettcannon]

@saponas what specifically is the issue? Is it simply running your tests or writing them?

/cc @d3r3kk and @ericsnowcurrently who have been doing a bunch of testing writing lately.

[saponas]

Thanks! I feel kinda silly that I couldn't get this working. From VS Code I'm running "Launch Tests" and seeing a bunch of unit test run in the DEBUG CONSOLE. Then some unit test fail in the ProcessService suite. Then the last test that runs is SocketCallbackHandler. The system never gets down to the lint tests (which is where I added my unit tests). I was hoping there is something obvious I'm missing on how to run the unit tests.

[d3r3kk]

Hey @saponas, it can be a bit daunting to run our tests as there are so many of them. To weed them out a little, you can do the following:

For unit tests: Those tests that are in files named *.unit.test.ts, alter the launch.json file in the "Debug Unit Tests" section by setting the grep field:

    "args": [
        "timeout=60000",
        "grep=[The suite name of your unit test file]"
    ],

...this will only run the suite with the tests you care about during a test run (be sure to set the debugger to run the Debug Unit Tests launcher).

If you are running system tests (we call them system tests, others call them integration or otherwise) you can achieve the same thing by editing the src/test/index.ts file here:

https://github.com/Microsoft/vscode-python/blob/b328ba12331ed34a267e32e77e3e4b1eff235c13/src/test/index.ts#L21

...to something like this:

const grep = '[The suite name of your *test.ts file]'; // IS_CI_SERVER &&...

...and then use the Launch Tests debugger launcher as you are doing now. This will run only the suite you name in the grep.

And be sure to escape any grep-sensitive characters in your suite name!

[saponas]

Hi @d3r3kk - Thanks for the help! That got me so I can run the linting system tests (including my system test). But it looks like they are all failing with not being able to find Python...

Python Extension: Linter 'pylint' is not installed. Please install it or select another linter". Error: spawn python ENOENT

Any tips on the setup for dev environment so that when I do "Launch Tests" it finds the Python environment? I have a conda environment with Python 2.7 that is selectable in the VS Code instance that I'm launching the tests from.

Here is the change I made to index.ts to get them running:

const grep = 'Linting \- General Tests';

Thanks!

[d3r3kk]

Oh yes, I'd forgotten about the Python interpreter that runs during testing. It will try and run the system default, unless you explicitly set the CI_PYTHON_PATH environment variable. Indeed, if you want to test against different versions of Python you must use this.

In the launch.json file, you can add the following to the Launch Tests setting:

    "env":{
        "CI_PYTHON_PATH": "/path/to/interpreter/of/choice/python"
    }

[d3r3kk]

(BTW: I am going to use this interaction to hopefully improve our docs on contributing...)

[saponas]

Thanks!!! That's exactly the info I needed! @brettcannon @d3r3kk

Added a system test to the PR. I tested that the test fails without this PR and passes after this PR. I also verified all the existing linting tests still pass.

[brettcannon]

Two fairly minor changes. Otherwise LGTM!

[brettcannon]

Typo in the name: threeLineLintsPath.

[brettcannon]

Could you add comments what the linter warnings are triggered by and an example of what the linter warning is? This is to prevent someone from accidentally changing the file and breaking the test's expectations. It will also help for us to know the tests is testing multi-line tests. 😉

[saponas]

@brettcannon Thanks! I addressed both your comments.

[brettcannon]

@saponas Thanks!