Skip to content

Conversation

karthiknadig
Copy link
Member

For #15357

The test already looked verified the files. It just needed a "dir" with the same name as a python binary. Added a empty file in such a dir.

Adding "skip tests" label because the test was just missing a entry in the FS layout.

@karthiknadig karthiknadig added the no-changelog No news entry required label Feb 9, 2021
const pythonBins: Set<string> = new Set();
for (const knownPath of knownPaths) {
const files = (await fsapi.readdir(knownPath))
const paths = (await fsapi.readdir(knownPath))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the following API we wouldn't need a lstat call,

raw = await fs.promises.readdir(dirname, { withFileTypes: true });

Or we can directly call getExecutablesDefault like windows path locator is doing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the comment below, on non-Windows the file type of each entry is preserved for free, so it doesn't run lstat.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes... forgot about this. Thanks.

@karrtikr karrtikr added the skip tests Updates to tests unnecessary label Feb 9, 2021
Copy link

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov-io
Copy link

Codecov Report

Merging #15367 (972649f) into main (36bb325) will decrease coverage by 0%.
The diff coverage is 92%.

@@          Coverage Diff           @@
##            main   #15367   +/-   ##
======================================
- Coverage     65%      65%   -1%     
======================================
  Files        559      559           
  Lines      26799    26801    +2     
  Branches    3868     3869    +1     
======================================
+ Hits       17447    17448    +1     
  Misses      8634     8634           
- Partials     718      719    +1     
Impacted Files Coverage Δ
...rc/client/pythonEnvironments/common/commonUtils.ts 58% <66%> (ø)
src/client/pythonEnvironments/common/posixUtils.ts 100% <100%> (ø)
...covery/locators/services/posixKnownPathsLocator.ts 100% <100%> (ø)
src/client/pythonEnvironments/base/info/env.ts 72% <0%> (-1%) ⬇️
...nts/base/locators/composite/environmentsReducer.ts 97% <0%> (ø)

@karthiknadig karthiknadig merged commit 52e67a2 into microsoft:main Feb 9, 2021
karthiknadig added a commit to karthiknadig/vscode-python that referenced this pull request Feb 10, 2021
…5367)

* Ignore directories with the same name as pyhton binaries

* Use withFileTypes instead of lstat
@karthiknadig karthiknadig deleted the issue15357 branch February 17, 2021 00:04
karthiknadig added a commit that referenced this pull request Feb 18, 2021
* Cherry pick fix for native notebook support into release branch (#15369)

* Fix problem with notebook apis not being used. (#15366)

* Update changelog

* Remove news file

* Add extraPaths support to JediLSP (#15365)

* Add extraPaths support

* Remove duplicate opt option

* Eslint cleanup

* Fix tests

* Ignore directories with the same name as python binaries (#15367)

* Ignore directories with the same name as pyhton binaries

* Use withFileTypes instead of lstat

* Correct pipenv activation provider when in discovery experiment (#15319)

* Correct pipenv activation provider when in discovery experiment

* Fix ESLint errors

* In PythonEnvsReducer.resolveEnv(), always fall back to the wrapped locator. (#15350)

fixes #15118

* Fix issue with missing interpreter info for some cases  (#15376)

* Use child process apis directly.

* Use raw API in process service

* Handle process cleanup

* Address sonar

* Refactor process service by pulling the raw process APIs out of the class

* Address comments

* Add reference to CVE-2020-16977 fixed in Oct 2020. (#15381)

* Fix CI failing with the number of constructor arguments error (#15347)

* Fix Command 'workbench.action.closeAllEditors' timed out failure on CI (#15322)

* Ensure environment variables provider can be created using standard classes (#15377)

* Ensure environment variables provider can be created using standard classes

* Fix unit tests

* Fix ESLint errors for environment variable provider (#15383)

* Fix problem with notebook apis not being used. (#15366)

* Show Python: Launch TensorBoard command in command palette (#15382) (#15386)

* Always register Launch TensorBoard command

* Put usage tracking behind experiment

* Cherry picks from main to release (#15421)

* Do not call activate the discovery component before registering all the classes (#15379)

* Do not attempt to activate discovery component before registering all the classes

* Add clarification comment

* Code reviews

* Skip windows store and shims paths when using known path locators (#15388)

* Skip windows store and shims paths when using known path locators

* Clean up and comments

* Tests

* Handle cases where envs variables might not be set

* Typo

Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

* Change "Pylance not installed" prompt to allow reverting to Jedi (#15420)

* Allow on suggestion refresh by default (#15430)

Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>

Co-authored-by: Rich Chiodo <rchiodo@users.noreply.github.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Eric Snow <eric.snow@microsoft.com>
Co-authored-by: Jim Griesmer <jimgries@microsoft.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>
karthiknadig added a commit that referenced this pull request Mar 8, 2021
* Cherry pick fix for native notebook support into release branch (#15369)

* Fix problem with notebook apis not being used. (#15366)

* Update changelog

* Remove news file

* Add extraPaths support to JediLSP (#15365)

* Add extraPaths support

* Remove duplicate opt option

* Eslint cleanup

* Fix tests

* Ignore directories with the same name as python binaries (#15367)

* Ignore directories with the same name as pyhton binaries

* Use withFileTypes instead of lstat

* Correct pipenv activation provider when in discovery experiment (#15319)

* Correct pipenv activation provider when in discovery experiment

* Fix ESLint errors

* In PythonEnvsReducer.resolveEnv(), always fall back to the wrapped locator. (#15350)

fixes #15118

* Fix issue with missing interpreter info for some cases  (#15376)

* Use child process apis directly.

* Use raw API in process service

* Handle process cleanup

* Address sonar

* Refactor process service by pulling the raw process APIs out of the class

* Address comments

* Add reference to CVE-2020-16977 fixed in Oct 2020. (#15381)

* Fix CI failing with the number of constructor arguments error (#15347)

* Fix Command 'workbench.action.closeAllEditors' timed out failure on CI (#15322)

* Ensure environment variables provider can be created using standard classes (#15377)

* Ensure environment variables provider can be created using standard classes

* Fix unit tests

* Fix ESLint errors for environment variable provider (#15383)

* Fix problem with notebook apis not being used. (#15366)

* Show Python: Launch TensorBoard command in command palette (#15382) (#15386)

* Always register Launch TensorBoard command

* Put usage tracking behind experiment

* Cherry picks from main to release (#15421)

* Do not call activate the discovery component before registering all the classes (#15379)

* Do not attempt to activate discovery component before registering all the classes

* Add clarification comment

* Code reviews

* Skip windows store and shims paths when using known path locators (#15388)

* Skip windows store and shims paths when using known path locators

* Clean up and comments

* Tests

* Handle cases where envs variables might not be set

* Typo

Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>

* Change "Pylance not installed" prompt to allow reverting to Jedi (#15420)

* Allow on suggestion refresh by default (#15430)

Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>

* Release final (#15433)

* Version update

* Change log updates

* TPN update

* Ensure pyenv virtual envs are not skipped when in discovery experiment (#15451)

* Ensure pyenv virtual envs are not skipped

* Add news

* Clean up

* Address comments

* Register Jedi regardless of what language server is configured (#15452)

* Register Jedi regardless of what language server is configured

* News entry

* Only call component adapter behind the discovery experiment (#15459)

* Update version and change log for point release (#15463)

* Version update

* Update change log

* Update start-up telemetry for Jedi LSP (#15419) (#15571)

* Version and change log update for point release (#15572)

Co-authored-by: Rich Chiodo <rchiodo@users.noreply.github.com>
Co-authored-by: Kartik Raj <karraj@microsoft.com>
Co-authored-by: Eric Snow <eric.snow@microsoft.com>
Co-authored-by: Jim Griesmer <jimgries@microsoft.com>
Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>
Co-authored-by: Kim-Adeline Miguel <51720070+kimadeline@users.noreply.github.com>
Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required skip tests Updates to tests unnecessary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants