Fix venv tests for terminal activation - #16747
Conversation
a41ab06 to
3bb18de
Compare
Kartik Raj (karrtikr)
changed the title
d9df831 to
c9c68b5
Compare
Kartik Raj (karrtikr)
left a comment
Kartik Raj (karrtikr)
left a comment
There was a problem hiding this comment.
Reasoning behind the fixes...
|
|
||
| // Produce a list of candidate binaries to be probed by exec'ing them. | ||
| async function* getCandidates() { | ||
| const customCondaPath = getPythonSetting<string>('condaPath'); |
There was a problem hiding this comment.
Conda tests were only failing on Windows. One of the reasons was that conda binary was not picked up from python.condaPath, due to which we were calling getCandidatesFromRegistry among other stuff, which was slow and tests timed out.
| test('Should activate with conda', async () => { | ||
| // Powershell does not work with conda by default, hence use cmd. | ||
| await terminalSettings.update( | ||
| 'integrated.shell.windows', |
There was a problem hiding this comment.
VSCode deprecated the old setting, which is when the tests started failing as conda tests needs cmd to run, if powershell is not configured.
| } | ||
| & $condaPythonPath ./build/ci/addEnvPath.py ${{env.PYTHON_VIRTUAL_ENVS_LOCATION}} condaExecPath $condaExecPath | ||
| & $condaPythonPath ./build/ci/addEnvPath.py ${{env.PYTHON_VIRTUAL_ENVS_LOCATION}} condaPath | ||
| & $condaExecPath init --all |
There was a problem hiding this comment.
Initialize all shells for conda activation (including powershell), so conda activation tests work even when not using cmd. We do not want to rely on this too much though (conda 🤷♂️) so still try to use cmd in the tests.
There was a problem hiding this comment.
Does this addition need to be repeated in all the workflow files that have a Prepare conda for venv tests step?
| let experiments: IExperimentService; | ||
| const sandbox = sinon.createSandbox(); | ||
| suiteSetup(async () => { | ||
| sandbox.stub(ExperimentHelpers, 'inDiscoveryExperiment').resolves(false); |
There was a problem hiding this comment.
This fixed the other non-conda venv tests.
Initially, when in discovery experiment, the resolver was slow and hence the tests were timing out. That has been fixed with https://github.com/microsoft/vscode-python-internalbacklog/issues/283 and hence tests now pass upon removing this line.
c9c68b5 to
6ddc852
Compare
| } | ||
| & $condaPythonPath ./build/ci/addEnvPath.py ${{env.PYTHON_VIRTUAL_ENVS_LOCATION}} condaExecPath $condaExecPath | ||
| & $condaPythonPath ./build/ci/addEnvPath.py ${{env.PYTHON_VIRTUAL_ENVS_LOCATION}} condaPath | ||
| & $condaExecPath init --all |
There was a problem hiding this comment.
Does this addition need to be repeated in all the workflow files that have a Prepare conda for venv tests step?
3 participants
Closes https://github.com/microsoft/vscode-python-internalbacklog/issues/308 closes https://github.com/microsoft/vscode-python-internalbacklog/issues/275
Used investigation from #16535.