Skip to content

Commit 74253ef

Browse files
rchiodorchiodo
authored
More of the functional tests are failing (#14346)
* Splitting test log * Fix problem with kernels ports being reused * Make kernel launcher port round robin only for testing * Make formatters change only apply during testing * Add news entry * Apply black formatting * Code review feedback and skip flakey remote password test * Another flakey test * More CR feedback * Missed a spot * Some more log parser changes and try to get interrupt to be less flakey * Fix interrupt killing kernel and add more logging for export * More logging * See if updating fixes the problem * Dont delete temp files * Upload webview output to figure out trust failure * Add name to step * Try another way to upload * Upload doesn't seem to work * Try a different way to upload * Try without webview logging as this makes the test pass * Try fixing test another way. Any logging is making the test pass * Compile error
1 parent 66e0eca commit 74253ef

10 files changed

Lines changed: 52 additions & 32 deletions

File tree

‎.github/workflows/pr_datascience.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ jobs:
151151
with:
152152
github_token: ${{ secrets.GITHUB_TOKEN }}
153153
report_paths: ${{ env.TEST_RESULTS_GLOB }}
154-
check_name: Functional Test Report
154+
check_name: Functional Test Report ${{matrix.test-suite}}
155155
if: steps.test_functional_group.outcome == 'failure' && failure()
156156

157157
testsInVSCode:

‎.vscode/launch.json‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,9 @@
292292
// Remove `X` prefix and update path to test with real python interpreter (for DS functional tests).
293293
"XCI_PYTHON_PATH": "<Python Path>",
294294
// Remove 'X' prefix to dump output for debugger. Directory has to exist prior to launch
295-
"XDEBUGPY_LOG_DIR": "${workspaceRoot}/tmp/Debug_Output"
295+
"XDEBUGPY_LOG_DIR": "${workspaceRoot}/tmp/Debug_Output",
296+
// Remove 'X' prefix to dump webview redux action log
297+
"XVSC_PYTHON_WEBVIEW_LOG_FILE": "${workspaceRoot}/test-webview.log"
296298
},
297299
"outFiles": ["${workspaceFolder}/out/**/*.js", "!${workspaceFolder}/**/node_modules**/*"],
298300
"preLaunchTask": "Compile",

‎build/ci/scripts/runFunctionalTests.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,13 @@ async function generateGroups(files) {
6868

6969
async function runIndividualTest(extraArgs, file, index) {
7070
var subMochaFile = `${mochaBaseFile}_${index}_${path.basename(file)}${mochaFileExt}`;
71-
process.env['MOCHA_FILE'] = subMochaFile;
7271
var args = gatherArgs(extraArgs, file);
7372
console.log(`Running functional test for file ${file} ...`);
7473
var exitCode = await new Promise((resolve) => {
7574
// Spawn the sub node process
76-
var proc = child_process.fork('./node_modules/mocha/bin/_mocha', args);
75+
var proc = child_process.fork('./node_modules/mocha/bin/_mocha', args, {
76+
env: { ...process.env, MOCHA_FILE: subMochaFile }
77+
});
7778
proc.on('exit', resolve);
7879
});
7980

‎pythonFiles/vscode_datascience_helpers/jupyter_daemon.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,15 @@ def _print_kernel_list_json(self):
140140
sys.stdout.flush()
141141

142142
def _convert(self, args):
143-
self.log.info("nbconvert")
143+
self.log.info("Starting nbconvert wirth args %s", args)
144144
from nbconvert import nbconvertapp as app
145145

146-
sys.argv = [""] + args
147-
app.main()
146+
try:
147+
sys.argv = [""] + args
148+
app.main()
149+
except Exception as e:
150+
self.log.info("Nbconvert error: %s", e)
151+
raise
148152

149153
def _start_notebook(self, args, cwd, env):
150154
from notebook import notebookapp as app

‎pythonFiles/vscode_datascience_helpers/tests/logParser.py‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from io import TextIOWrapper
12
import sys
23
import argparse
34
import os
@@ -18,13 +19,25 @@
1819
)
1920
ansi_escape = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")
2021
pid_regex = re.compile(r"(\d+).*")
22+
timestamp_regex = re.compile(r"\d{4}-\d{2}-\d{2}T.*\dZ")
23+
24+
25+
def stripTimestamp(line: str):
26+
match = timestamp_regex.match(line)
27+
if match:
28+
return line[match.end() :]
29+
return line
30+
31+
32+
def readStripLines(f: TextIOWrapper):
33+
return map(stripTimestamp, f.readlines())
2134

2235

2336
def printTestOutput(testlog):
2437
# Find all the lines that don't have a PID in them. These are the test output
2538
p = Path(testlog[0])
2639
with p.open() as f:
27-
for line in f.readlines():
40+
for line in readStripLines(f):
2841
stripped = line.strip()
2942
if len(stripped) > 2 and stripped[0] == "\x1B" and stripped[1] == "[":
3043
print(line.rstrip()) # Should be a test line as it has color encoding
@@ -38,15 +51,14 @@ def splitByPid(testlog):
3851
logs = {}
3952
pid = None
4053
with p.open() as f:
41-
for line in f.readlines():
54+
for line in readStripLines(f):
4255
stripped = ansi_escape.sub("", line.strip())
43-
# See if starts with a pid
44-
if len(stripped) > 0 and stripped[0] <= "9" and stripped[0] >= "0":
56+
if len(stripped) > 0:
4557
# Pull out the pid
4658
match = pid_regex.match(stripped)
4759

4860
# Pids are at least two digits
49-
if match != None and len(match.group(1)) > 2:
61+
if match and len(match.group(1)) > 2:
5062
# Pid is found
5163
pid = int(match.group(1))
5264

‎src/client/datascience/export/exportBase.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ export class ExportBase implements IExport {
5656
'--output',
5757
path.basename(tempTarget.filePath),
5858
'--output-dir',
59-
path.dirname(tempTarget.filePath)
59+
path.dirname(tempTarget.filePath),
60+
'--debug'
6061
];
6162
const result = await service.execModule('jupyter', ['nbconvert'].concat(args), {
6263
throwOnStdErr: false,

‎src/test/datascience/dataScienceIocContainer.ts‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@
33
//tslint:disable:trailing-comma no-any
44
import * as child_process from 'child_process';
55
import { ReactWrapper } from 'enzyme';
6-
import * as fs from 'fs-extra';
7-
import * as glob from 'glob';
86
import { interfaces } from 'inversify';
97
import * as os from 'os';
108
import * as path from 'path';
@@ -27,7 +25,6 @@ import {
2725
import * as vsls from 'vsls/vscode';
2826
import { KernelDaemonPool } from '../../client/datascience/kernel-launcher/kernelDaemonPool';
2927

30-
import { promisify } from 'util';
3128
import { LanguageServerExtensionActivationService } from '../../client/activation/activationService';
3229
import { LanguageServerDownloader } from '../../client/activation/common/downloader';
3330
import { JediExtensionActivator } from '../../client/activation/jedi';
@@ -500,18 +497,6 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
500497
// Make sure to disable all command handling during dispose. Don't want
501498
// anything to startup again.
502499
this.commandManager.dispose();
503-
try {
504-
// Make sure to delete any temp files written by native editor storage
505-
const globPr = promisify(glob);
506-
const tempLocation = os.tmpdir;
507-
const tempFiles = await globPr(`${tempLocation}/*.ipynb`);
508-
if (tempFiles && tempFiles.length) {
509-
await Promise.all(tempFiles.map((t) => fs.remove(t)));
510-
}
511-
} catch (exc) {
512-
// tslint:disable-next-line: no-console
513-
console.log(`Exception on cleanup: ${exc}`);
514-
}
515500
await this.asyncRegistry.dispose();
516501
await super.dispose();
517502
this.disposed = true;

‎src/test/datascience/notebook.functional.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -680,8 +680,8 @@ suite('DataScience notebook tests', () => {
680680
try {
681681
importer.dispose();
682682
temp.dispose();
683-
} catch {
684-
// Don't care if they don't delete
683+
} catch (exc) {
684+
console.log(exc);
685685
}
686686
}
687687
});

‎src/test/datascience/raw-kernel/rawKernel.functional.test.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,23 @@ suite('DataScience raw kernel tests', () => {
120120
// Hence timeout is a test failure.
121121
const longCellExecutionRequest = requestExecute(
122122
rawKernel,
123-
'import time\nfor i in range(300):\n time.sleep(1)',
123+
`
124+
import time
125+
import sys
126+
for i in range(3000):
127+
sys.stdout.write('.')
128+
sys.stdout.flush()
129+
time.sleep(0.1)
130+
sys.stdout.write('\\\r')`,
124131
executionStarted
125132
);
126133

127134
// Wait until the execution has started (cuz we cannot interrupt until exec has started).
128135
await executionStarted.promise;
129136

137+
// Give it a bit to start running
138+
await sleep(300);
139+
130140
// Then throw the interrupt
131141
await rawKernel.interrupt();
132142

‎src/test/datascience/trustedNotebooks.functional.test.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import * as chaiAsPromised from 'chai-as-promised';
66
import { ReactWrapper } from 'enzyme';
77
import * as fs from 'fs-extra';
88
import { Disposable } from 'vscode';
9+
import { sleep } from '../../client/common/utils/async';
910
import { noop } from '../../client/common/utils/misc';
1011
import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes';
1112
import { INotebookEditor, INotebookEditorProvider, ITrustService } from '../../client/datascience/types';
@@ -440,10 +441,14 @@ suite('DataScience Notebook trust', () => {
440441
// Reopen
441442
const newNativeEditor = await openEditor(ioc, baseFile, notebookFile.filePath);
442443
const newWrapper = newNativeEditor.mount.wrapper;
444+
assert.ok(newNativeEditor.editor.model.isTrusted, 'Editor did not open as trusted');
445+
446+
// Wait a bit. UI doesn't seem to update right away
447+
await sleep(500);
443448

444449
// Verify notebook is now trusted
445450
const after = newWrapper.find(TrustMessage);
446-
assert.equal(after.text(), 'Trusted');
451+
assert.equal(after.text(), 'Trusted', 'Notebook UI not reflecting trust state');
447452
});
448453
});
449454
});

0 commit comments

Comments
 (0)