Skip to content

Commit 8b31e74

Browse files
committed
address all the feedback.
1 parent f80b94a commit 8b31e74

33 files changed

+219
-211
lines changed

build/fix-telemetry-imports.mjs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ function fixTelemetryImports(content) {
3838
// Fix: './telemetry/index' -> './telemetry' (top-level telemetry.ts file)
3939
// Fix: '../telemetry/index' -> '../telemetry'
4040
// Fix: '../../telemetry/index' -> '../../telemetry' etc.
41-
const pattern = /(from\s+['"])((?:\.\.\/?)+)telemetry\/index(['"'])/g;
41+
const pattern = /(from\s+['"])((?:\.\.?\/)+)telemetry\/index(['"'])/g;
4242
modified = modified.replace(pattern, (match, before, dots, after) => {
4343
changeCount++;
4444
return `${before}${dots}telemetry${after}`;
4545
});
4646

4747
// Fix the double path: './platform/telemetry/telemetry/index' -> './platform/telemetry'
48-
const doublePath = /(from\s+['"])((?:\.\.\/?)+)platform\/telemetry\/telemetry\/index(['"'])/g;
48+
const doublePath = /(from\s+['"])((?:\.\.?\/)+)platform\/telemetry\/telemetry\/index(['"'])/g;
4949
modified = modified.replace(doublePath, (match, before, dots, after) => {
5050
changeCount++;
5151
return `${before}${dots}platform/telemetry${after}`;
@@ -80,7 +80,7 @@ async function main() {
8080
console.log(`🔗 Fixed ${totalImportsFixed} telemetry import${totalImportsFixed !== 1 ? 's' : ''}`);
8181
}
8282

83-
main().catch(error => {
83+
main().catch((error) => {
8484
console.error('❌ Error:', error);
8585
process.exit(1);
8686
});

build/preDebugWebTest.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ import path from 'node:path';
66
import { startJupyter } from './preLaunchWebTest.js';
77
import jsonc from 'jsonc-parser';
88
import { fileURLToPath } from 'node:url';
9-
import { dirname } from 'node:path';
109

1110
const __filename = fileURLToPath(import.meta.url);
12-
const __dirname = dirname(__filename);
11+
const __dirname = path.dirname(__filename);
1312

1413
const settingsFile = path.join(__dirname, '..', 'src', 'test', 'datascience', '.vscode', 'settings.json');
1514

build/preLaunchWebTest.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@ import path from 'node:path';
55
import jupyterServer from '../out/test/datascience/jupyterServer.node.js';
66
import fs from 'fs-extra';
77
import { fileURLToPath } from 'node:url';
8-
import { dirname } from 'node:path';
98

109
const __filename = fileURLToPath(import.meta.url);
11-
const __dirname = dirname(__filename);
10+
const __dirname = path.dirname(__filename);
1211

1312
export async function startJupyter(detached) {
1413
const server = jupyterServer.JupyterServer.instance;

build/remove-js-extensions.mjs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ async function getAllTsFiles(dir) {
2323
if (!['node_modules', 'out', 'dist', '.git', '.vscode', 'resources'].includes(entry.name)) {
2424
files.push(...(await getAllTsFiles(fullPath)));
2525
}
26-
} else if (entry.name.endsWith('.ts') || entry.name.endsWith('.tsx')) {
26+
} else if ((entry.name.endsWith('.ts') || entry.name.endsWith('.tsx')) && !entry.name.endsWith('.d.ts')) {
2727
files.push(fullPath);
2828
}
2929
}
@@ -42,7 +42,7 @@ function removeJsExtensions(content) {
4242
// import ... from './path.js' or '../path.js'
4343
/(\bfrom\s+['"])(\.\/?[^'"]+)\.js(['"])/g,
4444
// import('./path.js') or import('../path.js')
45-
/(\bimport\s*\(\s*['"])(\.\/?[^'"]+)\.js(['"])/g,
45+
/(\bimport\s*\(\s*['"])(\.\/?[^'"]+)\.js(['"])/g
4646
];
4747

4848
for (const pattern of patterns) {
@@ -81,7 +81,7 @@ async function main() {
8181
console.log(`🔗 Removed ${totalExtensionsRemoved} .js extension${totalExtensionsRemoved !== 1 ? 's' : ''}`);
8282
}
8383

84-
main().catch(error => {
84+
main().catch((error) => {
8585
console.error('❌ Error:', error);
8686
process.exit(1);
8787
});

gulpfile.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,11 @@ import { dumpTestSummary } from './build/webTestReporter.js';
2121
import { Validator } from 'jsonschema';
2222
import * as common from './build/webpack/common.js';
2323
import * as jsonc from 'jsonc-parser';
24+
import { isCI } from './build/constants.js';
2425

2526
const __filename = fileURLToPath(import.meta.url);
2627
const __dirname = dirname(__filename);
2728

28-
const isCI = process.env.TF_BUILD !== undefined || process.env.GITHUB_ACTIONS === 'true';
29-
3029
gulp.task('createNycFolder', async (done) => {
3130
try {
3231
fs.mkdirSync(path.join(__dirname, '.nyc_output'));

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2209,7 +2209,7 @@
22092209
"displayName": "Deepnote Vega Chart Renderer",
22102210
"entrypoint": "./dist/webviews/webview-side/vegaRenderer/vegaRenderer.js",
22112211
"mimeTypes": [
2212-
"application/vnd.vega.v5+json"
2212+
"application/vnd.vega.v6+json"
22132213
],
22142214
"requiresMessaging": "optional"
22152215
},

src/kernels/deepnote/environments/deepnoteEnvironmentsView.unit.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,8 @@ suite('DeepnoteEnvironmentsView', () => {
360360
assert.strictEqual(capturedOptions.description, 'Environment for data science work');
361361
// Don't assert on pythonInterpreter.id as the helper functions transform it
362362
assert.ok(capturedOptions.pythonInterpreter, 'Python interpreter should be provided');
363+
assert.ok(capturedOptions.pythonInterpreter.uri, 'Python interpreter uri should be present');
364+
assert.ok(capturedOptions.pythonInterpreter.id, 'Python interpreter id should be present');
363365
assert.ok(capturedToken, 'Cancellation token should be provided');
364366

365367
// Verify success message was shown

src/kernels/execution/notebookUpdater.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,15 @@ function clearPendingChainedUpdatesForTestsImpl() {
5858
}
5959
}
6060

61-
// Export through a mutable object to allow stubbing in ESM tests
61+
// Export through a mutable object to allow stubbing in ESM tests.
62+
// This object is intentionally mutable - tests can replace these functions
63+
// by reassigning properties on notebookUpdaterUtils, which is necessary
64+
// because ESM modules are read-only and direct exports cannot be stubbed.
6265
export const notebookUpdaterUtils = {
6366
chainWithPendingUpdates: chainWithPendingUpdatesImpl,
6467
clearPendingChainedUpdatesForTests: clearPendingChainedUpdatesForTestsImpl
6568
};
6669

67-
// Keep original exports for backwards compatibility
70+
// Standalone exports for backwards compatibility
6871
export const chainWithPendingUpdates = notebookUpdaterUtils.chainWithPendingUpdates;
6972
export const clearPendingChainedUpdatesForTests = notebookUpdaterUtils.clearPendingChainedUpdatesForTests;

src/kernels/kernelDependencyService.node.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,9 @@ export class KernelDependencyService implements IKernelDependencyService {
114114
let promise = this.installPromises.get(key);
115115
let cancelTokenSource: CancellationTokenSource | undefined;
116116
if (!promise) {
117-
const cancelTokenSource = new CancellationTokenSource();
117+
cancelTokenSource = new CancellationTokenSource();
118118
const disposable = token.onCancellationRequested(() => {
119-
cancelTokenSource.cancel();
119+
cancelTokenSource!.cancel();
120120
disposable.dispose();
121121
});
122122
const install = async () => {
@@ -133,7 +133,7 @@ export class KernelDependencyService implements IKernelDependencyService {
133133
resource,
134134
kernelConnection.interpreter!,
135135
ui,
136-
cancelTokenSource,
136+
cancelTokenSource!,
137137
cannotChangeKernels,
138138
installWithoutPrompting
139139
);
@@ -148,7 +148,7 @@ export class KernelDependencyService implements IKernelDependencyService {
148148
promise
149149
.finally(() => {
150150
disposable.dispose();
151-
cancelTokenSource.dispose();
151+
cancelTokenSource!.dispose();
152152
})
153153
.catch(noop);
154154
this.installPromises.set(key, promise);

src/kernels/raw/session/rawSessionConnection.node.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,7 @@ export class RawSessionConnection implements Session.ISessionConnection {
4545
return this._kernel?.connectionStatus || 'disconnected';
4646
}
4747
get serverSettings(): ServerConnection.ISettings {
48-
// We do not expect anyone to use this. Hence return a setting thats now expected to work, but at least compiles.
49-
return ServerConnection.makeSettings({
50-
wsUrl: 'RAW'
51-
});
48+
throw new Error('serverSettings is not implemented for raw kernel connections');
5249
}
5350
get model(): Session.IModel {
5451
return {
@@ -166,16 +163,16 @@ export class RawSessionConnection implements Session.ISessionConnection {
166163
}
167164

168165
public setPath(_path: string): Promise<void> {
169-
throw new Error('Not yet implemented');
166+
throw new Error('setPath is not implemented for raw kernel connections');
170167
}
171168
public setName(_name: string): Promise<void> {
172-
throw new Error('Not yet implemented');
169+
throw new Error('setName is not implemented for raw kernel connections');
173170
}
174171
public setType(_type: string): Promise<void> {
175-
throw new Error('Not yet implemented');
172+
throw new Error('setType is not implemented for raw kernel connections');
176173
}
177174
public changeKernel(_options: Partial<Kernel.IModel>): Promise<Kernel.IKernelConnection> {
178-
throw new Error('Not yet implemented');
175+
throw new Error('changeKernel is not implemented for raw kernel connections');
179176
}
180177

181178
private onIOPubMessage(_sender: Kernel.IKernelConnection, msg: KernelMessage.IIOPubMessage) {

0 commit comments

Comments
 (0)