-
Notifications
You must be signed in to change notification settings - Fork 294
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.
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
Fix interpreter tests #7410
Fix interpreter tests #7410
Changes from 39 commits
3d0e80b
7f934e9
00c498a
5903973
22978c0
18e6df1
96f13b4
65b185c
9574014
69e79dc
611a58f
4711e19
85fd924
95d3394
b9fd41b
2a1109e
62ff3db
1ed7b40
310521f
cfc7144
dc2bb66
4aa8c46
86c8142
fc21221
15af1b0
946dde4
4947189
66ca21f
1030254
9f87900
8703957
f73ab80
6b8eb52
ed2354d
49462a2
3a8e2a3
25bbee2
2040893
4252041
47ef368
dbba5f3
04ff288
002c4f0
5e16ec2
34295a2
982e074
6d940d5
3429ffc
80eca1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -468,6 +468,8 @@ jobs: | |
- name: Install Python Libs | ||
if: env.conda_python == '' | ||
run: | | ||
python --version | ||
python -c "import sys;print(sys.executable)" | ||
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade -r ./requirements.txt | ||
python -m pip --disable-pip-version-check install -r build/debugger-install-requirements.txt | ||
python ./pythonFiles/install_debugpy.py | ||
|
@@ -556,15 +558,15 @@ jobs: | |
check_name: Functional Test Report ${{matrix.os}} | ||
if: steps.test_functional_group.outcome == 'failure' && failure() | ||
|
||
# Mosts tests are commented, hence nothing runs, thus no coverage (if not commented, CI falls over as this code expects coverage files) | ||
# Upload unit test coverage reports for later use in the "reports" job. | ||
- name: Upload unit test coverage reports | ||
uses: actions/upload-artifact@v2 | ||
if: "(success() || failure()) && !contains(github.ref, 'refs/heads/release')" | ||
with: | ||
name: ${{runner.os}}-${{env.COVERAGE_REPORTS}} | ||
path: .nyc_output | ||
retention-days: 1 | ||
# # Mosts tests are commented, hence nothing runs, thus no coverage (if not commented, CI falls over as this code expects coverage files) | ||
# # Upload unit test coverage reports for later use in the "reports" job. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This throws error, and functional atests (old style) are almost non-existent, hence don't see the need to break CI just for coverage. |
||
# - name: Upload unit test coverage reports | ||
# uses: actions/upload-artifact@v2 | ||
# if: "(success() || failure()) && !contains(github.ref, 'refs/heads/release')" | ||
# with: | ||
# name: ${{runner.os}}-${{env.COVERAGE_REPORTS}} | ||
# path: .nyc_output | ||
# retention-days: 1 | ||
|
||
vscodeTests: | ||
name: VS Code Tests # These tests run with Python extension & real Jupyter | ||
|
@@ -734,6 +736,8 @@ jobs: | |
- name: Install Python Libs | ||
if: matrix.python != 'conda' && matrix.python != 'noPython' | ||
run: | | ||
python --version | ||
python -c "import sys;print(sys.executable)" | ||
python -m pip --disable-pip-version-check install -t ./pythonFiles/lib/python --no-cache-dir --implementation py --no-deps --upgrade -r ./requirements.txt | ||
python -m pip --disable-pip-version-check install -r build/debugger-install-requirements.txt | ||
python ./pythonFiles/install_debugpy.py | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,9 +14,11 @@ | |
import { inject, injectable, named } from 'inversify'; | ||
import { CancellationToken, Disposable, Event, EventEmitter, Memento, Uri } from 'vscode'; | ||
import { IApplicationShell, ICommandManager, IWorkspaceService } from '../common/application/types'; | ||
import { isCI } from '../common/constants'; | ||
import { trackPackageInstalledIntoInterpreter } from '../common/installer/productInstaller'; | ||
import { ProductNames } from '../common/installer/productNames'; | ||
import { InterpreterUri } from '../common/installer/types'; | ||
import { traceInfo, traceInfoIf } from '../common/logger'; | ||
import { | ||
GLOBAL_MEMENTO, | ||
IDisposableRegistry, | ||
|
@@ -119,6 +121,7 @@ export class PythonApiProvider implements IPythonApiProvider { | |
this.hooksRegistered = true; | ||
if (!pythonExtension.isActive) { | ||
await pythonExtension.activate(); | ||
traceInfoIf(isCI, 'Python extension Activated'); | ||
this.didActivatePython.fire(); | ||
} | ||
pythonExtension.exports.jupyter.registerHooks(); | ||
|
@@ -313,6 +316,8 @@ export class InterpreterSelector implements IInterpreterSelector { | |
return this.apiProvider.getApi().then((api) => api.getSuggestions(resource)); | ||
} | ||
} | ||
|
||
const interpreterCacheForCI = new Map<string, PythonEnvironment[]>(); | ||
// eslint-disable-next-line max-classes-per-file | ||
@injectable() | ||
export class InterpreterService implements IInterpreterService { | ||
|
@@ -344,12 +349,36 @@ export class InterpreterService implements IInterpreterService { | |
|
||
@captureTelemetry(Telemetry.InterpreterListingPerf) | ||
public getInterpreters(resource?: Uri): Promise<PythonEnvironment[]> { | ||
// Ensure resource is not undefined, Because of https://github.com/microsoft/vscode-jupyter/issues/7440 | ||
resource = | ||
(this.workspace.workspaceFolders?.length || 0) > 0 ? this.workspace.workspaceFolders![0].uri : undefined; | ||
this.hookupOnDidChangeInterpreterEvent(); | ||
return this.apiProvider.getApi().then((api) => api.getInterpreters(resource)); | ||
const promise = this.apiProvider.getApi().then((api) => api.getInterpreters(resource)); | ||
if (isCI) { | ||
promise | ||
.then((items) => { | ||
const current = interpreterCacheForCI.get(resource?.toString() || ''); | ||
const itemToStore = items; | ||
if ( | ||
current && | ||
(itemToStore === current || JSON.stringify(itemToStore) === JSON.stringify(current)) | ||
) { | ||
return; | ||
} | ||
interpreterCacheForCI.set(resource?.toString() || '', itemToStore); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like Python extension stuff doesn't get logged, this was crucial to figuring out what was going on. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've used the logs from Python extension a lot in the past, hence feel this is useful, but NOT necessary. Can remove if anyone things we don't need this, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't see an issue with leaving it in myself. |
||
traceInfo(`Interpreter list for ${resource?.toString()} is ${JSON.stringify(items)}`); | ||
}) | ||
.catch(noop); | ||
} | ||
return promise; | ||
} | ||
private workspaceCachedActiveInterpreter = new Map<string, Promise<PythonEnvironment | undefined>>(); | ||
@captureTelemetry(Telemetry.ActiveInterpreterListingPerf) | ||
public getActiveInterpreter(resource?: Uri): Promise<PythonEnvironment | undefined> { | ||
// Ensure resource is not undefined, Because of https://github.com/microsoft/vscode-jupyter/issues/7440 | ||
resource = | ||
(this.workspace.workspaceFolders?.length || 0) > 0 ? this.workspace.workspaceFolders![0].uri : undefined; | ||
traceInfoIf(isCI, `getActiveInterpreter in Python API for ${resource?.toString()}`); | ||
this.hookupOnDidChangeInterpreterEvent(); | ||
const workspaceId = this.workspace.getWorkspaceFolderIdentifier(resource); | ||
let promise = this.workspaceCachedActiveInterpreter.get(workspaceId); | ||
|
@@ -364,6 +393,13 @@ export class InterpreterService implements IInterpreterService { | |
this.workspaceCachedActiveInterpreter.delete(workspaceId); | ||
} | ||
}); | ||
if (isCI) { | ||
promise | ||
.then((item) => | ||
traceInfo(`Active Interpreter in Python API for ${resource?.toString()} is ${item?.path}`) | ||
) | ||
.catch(noop); | ||
} | ||
} | ||
} | ||
return promise; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Useful, this is how i figured some of the paths were different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No harm in leaving this.