-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Allow pytest to use correct interpreter from getActiveInterpreter #24233
Conversation
@@ -34,6 +35,7 @@ export class PytestTestDiscoveryAdapter implements ITestDiscoveryAdapter { | |||
private readonly outputChannel: ITestOutputChannel, | |||
private readonly resultResolver?: ITestResultResolver, | |||
private readonly envVarsService?: IEnvironmentVariablesProvider, | |||
private readonly interpreterService?: IInterpreterService, |
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.
wanted to avoid dependency injection here - bit difficult since there are listener and event handlers like onDidChangeInterpreters attached.
We could also partially avoid using configService for: path = this.configService.getSettings(resource).pythonPath;
- Rather just make an API to get getting with resource without configService would be cleaner.
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.
Don't need to plumb the service through all layers. We can just pass the interpreter into the various layers.
Opening PR #24250 - running into lots of merge conflict. |
Resolves: #24122
Related: #24190, #24127
I think the culprit was we were not passing in interpreter when we call createActivatedEnvironment.
/cc @eleanorjboyd