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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multiple linter support failing #2571

Closed
DonJayamanne opened this issue Sep 13, 2018 · 9 comments
Closed

Multiple linter support failing #2571

DonJayamanne opened this issue Sep 13, 2018 · 9 comments
Assignees
@ericsnowcurrently @d3r3kk
Labels
area-linting bug Issue identified by VS Code Team member as probable bug
Milestone
2019, week 1 - Ja...

Comments

@DonJayamanne
Copy link

Here's the output from the logs:

2018-09-12T23:52:33.0234844Z     ✖ Multiple linters -- error: No diagnostic messages.
2018-09-12T23:52:33.0235203Z 
2018-09-12T23:52:33.0236658Z ##[error]out\test\linters\lint.test.js(,): error : FAILED Linting - General Tests :: Multiple linters
2018-09-12T23:52:33.0237549Z ##[debug]Processed: ##vso[task.logissue type=error;sourcepath=D:\a\1\s\out\test\linters\lint.test.js;]FAILED Linting - General Tests :: Multiple linters
2018-09-12T23:52:33.0237762Z
@DonJayamanne DonJayamanne added bug Issue identified by VS Code Team member as probable bug needs PR labels Sep 13, 2018
@d3r3kk d3r3kk mentioned this issue Sep 18, 2018
Merged
6 tasks
@d3r3kk
Copy link

d3r3kk commented Sep 18, 2018

I've skipped this test for now, as there needs to be some real work done to sort out the root cause.

The branch linting_test_failure in my personal repo here contains some console.log messages that show the behaviour symptomatic of the failure (you will likely have to run it on Windows, under Python3.7, and mutliple times before you see the failure occur).

Essentially the trouble is that the Promise on the executeCommand('python.runLinter') is resolving before the command is actually complete, and it also seems to be returning more than a single time, judging from the console output I've seen.

@DonJayamanne DonJayamanne added the important Issue identified as high-priority label Dec 13, 2018
@brettcannon
Copy link
Member

Attached is my log. If I have just flake8 on it's fine, and if I just have pep8 turned on it's, fine. But as soon as I have both on I get a message saying pep8 isn't installed and an error about TypeError: Converting circular structure to JSON (log).

@brettcannon brettcannon changed the title Azure CI Test failure for Multiple Linters Multiple linter support failing Dec 13, 2018
@brettcannon brettcannon mentioned this issue Dec 13, 2018
Closed
@nullie
Copy link

nullie commented Dec 14, 2018 •
edited
Loading

https://github.com/Microsoft/vscode-python/blob/master/src/client/common/process/proc.ts#L172 method calls JSON.parse(JSON.stringify(options) (trying to make a deep copy?) , but fails on options.token._emitter._listeners

nullie added a commit to nullie/vscode-python that referenced this issue Dec 14, 2018
@nullie
Fix multiple linter support (microsoft#2571)
51aaa75
@nullie nullie mentioned this issue Dec 14, 2018
Merged
6 tasks
@d3r3kk d3r3kk added this to the 2018, week 50 - Dec sprint 2 milestone Dec 14, 2018
@d3r3kk d3r3kk added unplanned and removed important Issue identified as high-priority labels Dec 14, 2018
@d3r3kk
Copy link

d3r3kk commented Dec 14, 2018

We took a fix (#3702) that corrects the behaviour in the runtime, but now we need to actually fix the failure occurring in the tests. This is p0 for this release.

@d3r3kk d3r3kk self-assigned this Dec 14, 2018
d3r3kk pushed a commit that referenced this issue Dec 14, 2018
@nullie @d3r3kk
Fix multiple linter support (#2571) (#3702)
b0a54b5
d3r3kk pushed a commit that referenced this issue Dec 14, 2018
@nullie @d3r3kk
Fix multiple linter support (#2571) (#3702)
c16a769
d3r3kk added a commit that referenced this issue Dec 14, 2018
@d3r3kk
skipping unreliable linter test
898cc23 
- tracked by #2571
- correct news entry to address the appropriate issue
@nullie nullie mentioned this issue Dec 15, 2018
Closed
6 tasks
@nullie
Copy link

nullie commented Dec 19, 2018

Is the tests issue reproducible locally?

@brettcannon
Copy link
Member

@nullie yes

@d3r3kk d3r3kk mentioned this issue Dec 19, 2018
Merged
6 tasks
d3r3kk added a commit that referenced this issue Dec 20, 2018
@d3r3kk
Re-enable tests for multiple linters (#3756)
a139248 
Failing test is no longer able to repro locally.

For #2571
d3r3kk added a commit to d3r3kk/vscode-python that referenced this issue Dec 31, 2018
@d3r3kk
Skip failing test for multiple linters
fdeb0ef 
- notorious for time-outs
- we will mock linting output up instead
- microsoft#2571
d3r3kk added a commit to d3r3kk/vscode-python that referenced this issue Jan 1, 2019
@d3r3kk
Skip failing test for multiple linters
7229c69 
- notorious for time-outs
- we will mock linting output up instead
- microsoft#2571
d3r3kk added a commit to d3r3kk/vscode-python that referenced this issue Jan 4, 2019
@d3r3kk
Move 'Multiple linters' system test into its own file.
5cb6970 
Fix for microsoft#2571

- Multiple linters test just uses the app, no mocking necessary.
  - Thus it did not truly belong in linter.test.ts
- Only needed mocked linter output stub to be a good system test
d3r3kk added a commit to d3r3kk/vscode-python that referenced this issue Jan 4, 2019
@d3r3kk
Move 'Multiple linters' system test into its own file.
658282d 
Fix for microsoft#2571

- Multiple linters test just uses the app, no mocking necessary.
  - Thus it did not truly belong in linter.test.ts
- Only needed mocked linter output stub to be a good system test
@d3r3kk d3r3kk mentioned this issue Jan 4, 2019
Merged
6 tasks
d3r3kk added a commit that referenced this issue Jan 4, 2019
@d3r3kk
Move 'Multiple linters' system test into its own file (#3866)
2e5e9b5 
* Move 'Multiple linters' system test into its own file.

Fix for #2571

- Multiple linters test just uses the app, no mocking necessary.
  - Thus it did not truly belong in linter.test.ts
- Only needed mocked linter output stub to be a good system test
@ericsnowcurrently ericsnowcurrently self-assigned this Jan 7, 2019
@ericsnowcurrently
Copy link
Member

I'm going to validate this.

@ericsnowcurrently
Copy link
Member

@d3r3kk what are the steps to reproduce this? I'm not seeing the problem under the 2018.12.1 release. Is this WIndows-only?

@ericsnowcurrently
Copy link
Member

tests are passing now

@ghost ghost removed the needs PR label Jan 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Feb 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ericsnowcurrently ericsnowcurrently

@d3r3kk d3r3kk

Labels
area-linting bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Milestone
2019, week 1 - Jan Sprint 1
Development

No branches or pull requests
5 participants
@brettcannon @nullie @ericsnowcurrently @DonJayamanne @d3r3kk