Skip to content

switch to shell being true for execObservable #23271

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

Closed
wants to merge 11 commits into from
Closed

switch to shell being true for execObservable #23271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6cf4b11
switch to shell being true for execObservable
eleanorjboyd Apr 22, 2024
ecc5fc9
add better error logging
eleanorjboyd Apr 22, 2024
bc132f3
Merge branch 'main' into use-shell-for-execObservable
eleanorjboyd May 7, 2024
3a0d709
add json stringify to error messages
eleanorjboyd May 8, 2024
be366cc
fix error
eleanorjboyd May 8, 2024
72b6862
try with one shell changed
eleanorjboyd May 8, 2024
6c72463
attempt to remove uses statement
eleanorjboyd May 9, 2024
3d66fbc
try removing second location
eleanorjboyd May 9, 2024
151e7cd
try to separate different OS
eleanorjboyd May 9, 2024
146a02c
format
eleanorjboyd May 9, 2024
4f4270a
try with pwsh
eleanorjboyd May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions .github/workflows/pr-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,14 +288,21 @@ jobs:
working-directory: ${{ env.special-working-directory }}
if: matrix.test-suite == 'venv'

- name: Run single-workspace tests
- name: Run single-workspace tests on non-Linux
env:
CI_PYTHON_VERSION: ${{ matrix.python }}
uses: GabrielBB/[email protected]
with:
run: npm run testSingleWorkspace
working-directory: ${{ env.special-working-directory }}
if: matrix.test-suite == 'single-workspace'
run: npm run testSingleWorkspace
shell: pwsh
working-directory: ${{ env.special-working-directory }}
if: runner.os != 'Linux' && matrix.test-suite == 'single-workspace'

- name: Run single-workspace tests on Linux
env:
CI_PYTHON_VERSION: ${{ matrix.python }}
run: xvfb-run -a npm run testSingleWorkspace
shell: pwsh
working-directory: ${{ env.special-working-directory }}
if: runner.os == 'Linux' && matrix.test-suite == 'single-workspace'

- name: Run debugger tests
env:
Expand Down Expand Up @@ -476,13 +483,21 @@ jobs:
with:
run: npm run testSingleWorkspace:cover

- name: Run single-workspace tests
- name: Run single-workspace tests on non-Linux
env:
CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }}
CI_DISABLE_AUTO_SELECTION: 1
uses: GabrielBB/[email protected]
with:
run: npm run testSingleWorkspace:cover
run: npm run testSingleWorkspace:cover
shell: pwsh
if: runner.os != 'Linux'

- name: Run single-workspace tests on Linux
env:
CI_PYTHON_VERSION: ${{ env.PYTHON_VERSION }}
CI_DISABLE_AUTO_SELECTION: 1
run: xvfb-run -a npm run testSingleWorkspace:cover
shell: pwsh
if: runner.os == 'Linux'

# Enable these tests when coverage is setup for multiroot workspace tests
# - name: Run multi-workspace tests
Expand Down
1 change: 1 addition & 0 deletions src/client/testing/testController/unittest/testDiscoveryAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export class UnittestTestDiscoveryAdapter implements ITestDiscoveryAdapter {
throwOnStdErr: true,
outputChannel: options.outChannel,
env: mutableEnv,
shell: true,
};
// Create the Python environment in which to execute the command.
const creationOptions: ExecutionFactoryCreateWithEnvironmentOptions = {
Expand Down
1 change: 1 addition & 0 deletions src/client/testing/testController/unittest/testExecutionAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export class UnittestTestExecutionAdapter implements ITestExecutionAdapter {
throwOnStdErr: true,
outputChannel: options.outChannel,
env: mutableEnv,
shell: true,
};
// Create the Python environment in which to execute the command.
const creationOptions: ExecutionFactoryCreateWithEnvironmentOptions = {
Expand Down
58 changes: 39 additions & 19 deletions src/test/testing/common/testingAdapter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ suite('End to End Tests: test adapters', () => {
let callCount = 0;
// const deferredTillEOT = createTestingDeferred();
resultResolver._resolveDiscovery = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
actualData = payload;
return Promise.resolve();
Expand All @@ -188,7 +188,9 @@ suite('End to End Tests: test adapters', () => {
assert.strictEqual(
actualData.status,
'success',
`Expected status to be 'success' instead status is ${actualData.status}`,
`Expected status to be 'success' instead status is ${actualData.status}, error: ${
actualData.error
}, payload: ${JSON.stringify(actualData)}`,
);
// 2. Confirm no errors
assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field");
Expand All @@ -210,7 +212,7 @@ suite('End to End Tests: test adapters', () => {
resultResolver = new PythonResultResolver(testController, unittestProvider, workspaceUri);
let callCount = 0;
resultResolver._resolveDiscovery = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
actualData = payload;
return Promise.resolve();
Expand All @@ -232,7 +234,9 @@ suite('End to End Tests: test adapters', () => {
assert.strictEqual(
actualData.status,
'success',
`Expected status to be 'success' instead status is ${actualData.status}`,
`Expected status to be 'success' instead status is ${actualData.status}, error: ${
actualData.error
}, payload: ${JSON.stringify(actualData)}`,
);
// 2. Confirm no errors
assert.strictEqual(actualData.error, undefined, "Expected no errors in 'error' field");
Expand Down Expand Up @@ -274,7 +278,9 @@ suite('End to End Tests: test adapters', () => {
assert.strictEqual(
actualData.status,
'success',
`Expected status to be 'success' instead status is ${actualData.status}`,
`Expected status to be 'success' instead status is ${actualData.status}, error: ${
actualData.error
}, payload: ${JSON.stringify(actualData)}`,
); // 2. Confirm no errors
assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field");
// 3. Confirm tests are found
Expand Down Expand Up @@ -309,7 +315,7 @@ suite('End to End Tests: test adapters', () => {
resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri);
let callCount = 0;
resultResolver._resolveDiscovery = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
actualData = payload;
return Promise.resolve();
Expand All @@ -330,7 +336,9 @@ suite('End to End Tests: test adapters', () => {
assert.strictEqual(
actualData.status,
'success',
`Expected status to be 'success' instead status is ${actualData.status}`,
`Expected status to be 'success' instead status is ${actualData.status}, error: ${
actualData.error
}, payload: ${JSON.stringify(actualData)}`,
); // 2. Confirm no errors
assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field");
// 3. Confirm tests are found
Expand Down Expand Up @@ -398,7 +406,7 @@ suite('End to End Tests: test adapters', () => {
resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri);
let callCount = 0;
resultResolver._resolveDiscovery = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
actualData = payload;
return Promise.resolve();
Expand All @@ -419,7 +427,9 @@ suite('End to End Tests: test adapters', () => {
assert.strictEqual(
actualData.status,
'success',
`Expected status to be 'success' instead status is ${actualData.status}`,
`Expected status to be 'success' instead status is ${actualData.status}, error: ${
actualData.error
}, payload: ${JSON.stringify(actualData)}`,
); // 2. Confirm no errors
assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field");
// 3. Confirm tests are found
Expand Down Expand Up @@ -474,7 +484,7 @@ suite('End to End Tests: test adapters', () => {
resultResolver = new PythonResultResolver(testController, pytestProvider, workspaceUri);
let callCount = 0;
resultResolver._resolveDiscovery = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
actualData = payload;
return Promise.resolve();
Expand All @@ -497,7 +507,9 @@ suite('End to End Tests: test adapters', () => {
assert.strictEqual(
actualData.status,
'success',
`Expected status to be 'success' instead status is ${actualData.status}`,
`Expected status to be 'success' instead status is ${actualData.status}, error: ${
actualData.error
}, payload: ${JSON.stringify(actualData)}`,
); // 2. Confirm no errors
assert.strictEqual(actualData.error?.length, 0, "Expected no errors in 'error' field");
// 3. Confirm tests are found
Expand All @@ -513,14 +525,16 @@ suite('End to End Tests: test adapters', () => {
let failureOccurred = false;
let failureMsg = '';
resultResolver._resolveExecution = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
// the payloads that get to the _resolveExecution are all data and should be successful.
try {
assert.strictEqual(
payload.status,
'success',
`Expected status to be 'success', instead status is ${payload.status}`,
`Expected status to be 'success' instead status is ${payload.status}, error: ${
payload.error
}, payload: ${JSON.stringify(payload)}`,
);
assert.ok(payload.result, 'Expected results to be present');
} catch (err) {
Expand Down Expand Up @@ -592,14 +606,16 @@ suite('End to End Tests: test adapters', () => {
let failureOccurred = false;
let failureMsg = '';
resultResolver._resolveExecution = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
// the payloads that get to the _resolveExecution are all data and should be successful.
try {
const validStatuses = ['subtest-success', 'subtest-failure'];
assert.ok(
validStatuses.includes(payload.status),
`Expected status to be one of ${validStatuses.join(', ')}, but instead status is ${payload.status}`,
`Expected status to be one of ${validStatuses.join(', ')}, but instead status is ${
payload.status
}, error: ${payload.error}`,
);
assert.ok(payload.result, 'Expected results to be present');
} catch (err) {
Expand Down Expand Up @@ -668,14 +684,16 @@ suite('End to End Tests: test adapters', () => {
let failureOccurred = false;
let failureMsg = '';
resultResolver._resolveExecution = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
// the payloads that get to the _resolveExecution are all data and should be successful.
try {
assert.strictEqual(
payload.status,
'success',
`Expected status to be 'success', instead status is ${payload.status}`,
`Expected status to be 'success' instead status is ${payload.status}, error: ${
payload.error
}, payload: ${JSON.stringify(payload)}`,
);
assert.ok(payload.result, 'Expected results to be present');
} catch (err) {
Expand Down Expand Up @@ -757,14 +775,16 @@ suite('End to End Tests: test adapters', () => {
let failureOccurred = false;
let failureMsg = '';
resultResolver._resolveExecution = async (payload, _token?) => {
traceLog(`resolveDiscovery ${payload}`);
traceLog(`resolveDiscovery ${JSON.stringify(payload)}`);
callCount = callCount + 1;
// the payloads that get to the _resolveExecution are all data and should be successful.
try {
assert.strictEqual(
payload.status,
'success',
`Expected status to be 'success', instead status is ${payload.status}`,
`Expected status to be 'success' instead status is ${payload.status}, error: ${
payload.error
}, payload: ${JSON.stringify(payload)}`,
);
assert.ok(payload.result, 'Expected results to be present');
} catch (err) {
Expand Down
Loading