-
Notifications
You must be signed in to change notification settings - Fork 57
test: add cross-platform package manager lifecycle coverage #1717
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
base: main
Are you sure you want to change the base?
Changes from all commits
f763ea4
cb05041
e1f3481
df6d1a2
efe2f9c
cd739a5
13cd44d
6f93cf0
a435996
a9de0e7
8fe8255
0462436
148891f
6764006
ebb222d
0601d34
26250c5
76c45af
a0fe872
afac6d2
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 |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ import { | |
| ResolveEnvironmentContext, | ||
| SetEnvironmentScope, | ||
| } from '../api'; | ||
| import { PackageVersionLookupNotSupportedError } from '../common/errors/NotSupportedError'; | ||
| import { traceError, traceInfo } from '../common/logging'; | ||
| import { pickEnvironmentManager } from '../common/pickers/managers'; | ||
| import { timeout } from '../common/utils/asyncUtils'; | ||
|
|
@@ -326,7 +327,9 @@ export class PythonEnvironmentApiImpl implements PythonEnvironmentApi { | |
| await waitForEnvManagerId([context.envId.managerId]); | ||
| const manager = this.envManagers.getPackageManager(context); | ||
| if (!manager) { | ||
| return Promise.resolve(undefined); | ||
| throw new PackageVersionLookupNotSupportedError( | ||
| `No package manager supports version lookup for: ${context.envId.id}`, | ||
| ); | ||
| } | ||
|
Contributor
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 still changes the public contract from resolving [verified] |
||
| return manager.getPackageAvailableVersions(context, packageName); | ||
|
Contributor
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 changes the public API from resolving [verified]
Contributor
Author
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 is addressed in #1728 |
||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.