Skip to content

Run extension-internal scripts "isolated". #10941

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
Merged
merged 4 commits into from
Apr 8, 2020
Merged

Run extension-internal scripts "isolated". #10941

merged 4 commits into from
Apr 8, 2020

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Apr 2, 2020 •
edited
Loading

For #10681.

This makes it so that none of the extension's internal scripts get run with sys.path[0] set to CWD. Note that test adapter script is the only one that is not run isolated.

@ericsnowcurrently ericsnowcurrently added the no-changelog No news entry required label Apr 2, 2020
@ericsnowcurrently ericsnowcurrently changed the title Run our internal scripts "isolated". Run extension-internal scripts "isolated". Apr 2, 2020
@ericsnowcurrently ericsnowcurrently mentioned this pull request Apr 2, 2020
Merged
@codecov-io
Copy link

codecov-io commented Apr 2, 2020 •
edited
Loading

Codecov Report

Merging #10941 into master will decrease coverage by 0.04%.
The diff coverage is 52.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10941      +/-   ##
==========================================
- Coverage   60.78%   60.74%   -0.05%     
==========================================
  Files         585      585              
  Lines       31924    31944      +20     
  Branches     4523     4524       +1     
==========================================
- Hits        19406    19405       -1     
- Misses      11530    11549      +19     
- Partials      988      990       +2
Impacted Files Coverage Δ
...t/common/process/internal/scripts/testing_tools.ts 100.00% <ø> (ø)
...ess/internal/scripts/vscode_datascience_helpers.ts 33.33% <14.28%> (ø)
...rc/client/common/process/internal/scripts/index.ts 70.12% <75.00%> (+0.79%) ⬆️
src/datascience-ui/react-common/arePathsSame.ts 75.00% <0.00%> (-12.50%) ⬇️
src/client/common/utils/platform.ts 64.70% <0.00%> (-11.77%) ⬇️
src/client/linters/pydocstyle.ts 86.66% <0.00%> (-2.23%) ⬇️
src/client/common/application/applicationShell.ts 6.38% <0.00%> (-1.73%) ⬇️
src/client/datascience/debugLocationTracker.ts 76.56% <0.00%> (-1.57%) ⬇️
...client/datascience/kernel-launcher/kernelFinder.ts 75.70% <0.00%> (-0.94%) ⬇️
src/client/common/process/proc.ts 14.49% <0.00%> (-0.73%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f29704...9ee3953. Read the comment docs.

karthiknadig
karthiknadig requested changes Apr 6, 2020
View reviewed changes
@karthiknadig karthiknadig requested a review from rchiodo April 6, 2020 04:03
@karthiknadig
Copy link
Member

@rchiodo There are changes to how we execute internal scripts. Please have a look.

rchiodo
rchiodo previously requested changes Apr 6, 2020
View reviewed changes
Copy link

@rchiodo rchiodo left a comment

Choose a reason for hiding this comment

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

🕐

DonJayamanne
DonJayamanne previously requested changes Apr 6, 2020
View reviewed changes
@ericsnowcurrently ericsnowcurrently force-pushed the internal-tools-isolated branch 3 times, most recently from 936cb2d to d8a6cf3 Compare April 7, 2020 15:59
@rchiodo rchiodo dismissed their stale review April 7, 2020 16:01

revoking review

karthiknadig
karthiknadig reviewed Apr 7, 2020
View reviewed changes
Copy link
Member

@karthiknadig karthiknadig left a comment

Choose a reason for hiding this comment

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

My only comment is to update move the contents of internal.ts into separate modules under internals rather than using namespaces.

@ericsnowcurrently ericsnowcurrently force-pushed the internal-tools-isolated branch from d8a6cf3 to 9d1fdc3 Compare April 7, 2020 19:29
@ericsnowcurrently ericsnowcurrently removed the request for review from karrtikr April 7, 2020 19:48
@ericsnowcurrently ericsnowcurrently force-pushed the internal-tools-isolated branch 2 times, most recently from 9ee3953 to 32715fe Compare April 8, 2020 17:06
@ericsnowcurrently ericsnowcurrently force-pushed the internal-tools-isolated branch from 32715fe to f48bb30 Compare April 8, 2020 17:50
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 8, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ericsnowcurrently ericsnowcurrently merged commit 77bf701 into microsoft:master Apr 8, 2020
@ericsnowcurrently ericsnowcurrently deleted the internal-tools-isolated branch April 8, 2020 18:17
@lock lock bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@karrtikr karrtikr karrtikr left review comments

@karthiknadig karthiknadig karthiknadig approved these changes

@DonJayamanne DonJayamanne DonJayamanne left review comments

@rchiodo rchiodo rchiodo left review comments

Assignees
No one assigned
Labels
no-changelog No news entry required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ericsnowcurrently @codecov-io @karthiknadig @DonJayamanne @karrtikr @rchiodo