Skip to content

Commit 5923099

Browse files
committed
fix(vscode): finish activation after telemetry restart recovery
Signed-off-by: Vamsi-klu <nrvamsi13@gmail.com>
1 parent dcc7995 commit 5923099

2 files changed

Lines changed: 167 additions & 4 deletions

File tree

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
import type { ExtensionContext } from 'vscode'
5+
import { err, ok } from '@bus/result'
6+
7+
const mocks = vi.hoisted(() => ({
8+
telemetryListener: undefined as ((enabled: boolean) => unknown) | undefined,
9+
start: vi.fn(),
10+
restart: vi.fn(),
11+
registerCommand: vi.fn<(command: string) => { dispose(): void }>(() => ({
12+
dispose: vi.fn(),
13+
})),
14+
registerContentProvider: vi.fn(() => ({ dispose: vi.fn() })),
15+
registerViewProvider: vi.fn(() => ({ dispose: vi.fn() })),
16+
setupTestController: vi.fn(() => ({ dispose: vi.fn() })),
17+
testController: { dispose: vi.fn() },
18+
handleError: vi.fn(),
19+
}))
20+
21+
vi.mock('vscode', () => ({
22+
env: {
23+
onDidChangeTelemetryEnabled: (listener: (enabled: boolean) => unknown) => {
24+
mocks.telemetryListener = listener
25+
return { dispose: vi.fn() }
26+
},
27+
},
28+
authentication: {
29+
registerAuthenticationProvider: vi.fn(() => ({ dispose: vi.fn() })),
30+
},
31+
commands: { registerCommand: mocks.registerCommand },
32+
workspace: {
33+
registerTextDocumentContentProvider: mocks.registerContentProvider,
34+
onDidSaveTextDocument: vi.fn(() => ({ dispose: vi.fn() })),
35+
},
36+
window: { registerWebviewViewProvider: mocks.registerViewProvider },
37+
}))
38+
39+
vi.mock('./lsp/lsp', () => ({
40+
LSPClient: class {
41+
start = mocks.start
42+
restart = mocks.restart
43+
hasCompletionCapability = () => true
44+
},
45+
}))
46+
vi.mock('./utilities/common/vscodeapi', () => ({
47+
createOutputChannel: vi.fn(() => ({ dispose: vi.fn() })),
48+
onDidChangeConfiguration: vi.fn(() => ({ dispose: vi.fn() })),
49+
registerCommand: mocks.registerCommand,
50+
}))
51+
vi.mock('./utilities/common/log', () => ({
52+
registerLogger: vi.fn(() => ({ dispose: vi.fn() })),
53+
traceInfo: vi.fn(),
54+
traceVerbose: vi.fn(),
55+
traceError: vi.fn(),
56+
}))
57+
vi.mock('./utilities/common/python', () => ({
58+
onDidChangePythonInterpreter: vi.fn(() => ({ dispose: vi.fn() })),
59+
}))
60+
vi.mock('./utilities/errors', () => ({ handleError: mocks.handleError }))
61+
vi.mock('./auth/auth', () => ({
62+
AuthenticationProviderTobikoCloud: vi.fn(),
63+
}))
64+
vi.mock('./tests/tests', () => ({
65+
controller: mocks.testController,
66+
setupTestController: mocks.setupTestController,
67+
}))
68+
vi.mock('./providers/renderedModelProvider', () => ({
69+
RenderedModelProvider: Object.assign(vi.fn(), {
70+
getScheme: () => 'sqlmesh-rendered',
71+
}),
72+
}))
73+
vi.mock('./webviews/lineagePanel', () => ({ LineagePanel: vi.fn() }))
74+
vi.mock('./completion/completion', () => ({
75+
selector: [],
76+
completionProvider: vi.fn(),
77+
}))
78+
vi.mock('./commands/format', () => ({ format: vi.fn() }))
79+
vi.mock('./commands/signout', () => ({ signOut: vi.fn() }))
80+
vi.mock('./commands/signin', () => ({ signIn: vi.fn() }))
81+
vi.mock('./commands/signinSpecifyFlow', () => ({ signInSpecifyFlow: vi.fn() }))
82+
vi.mock('./commands/renderModel', () => ({
83+
renderModel: vi.fn(),
84+
reRenderModelForSourceFile: vi.fn(),
85+
}))
86+
vi.mock('./commands/stop', () => ({ stop: vi.fn() }))
87+
vi.mock('./commands/printEnvironment', () => ({ printEnvironment: vi.fn() }))
88+
vi.mock('./commands/tableDiff', () => ({ showTableDiff: vi.fn() }))
89+
90+
describe('activation after a telemetry change during startup', () => {
91+
beforeEach(() => {
92+
vi.resetModules()
93+
vi.clearAllMocks()
94+
mocks.telemetryListener = undefined
95+
mocks.restart.mockResolvedValue(ok(undefined))
96+
mocks.handleError.mockResolvedValue(undefined)
97+
})
98+
99+
it.each([false, true])(
100+
'registers the full extension after a successful retry, initial start failed: %s',
101+
async initialStartFailed => {
102+
mocks.start.mockImplementationOnce(() => {
103+
mocks.telemetryListener?.(false)
104+
return Promise.resolve(
105+
initialStartFailed
106+
? err({ type: 'sqlmesh_lsp_not_found' })
107+
: ok(undefined),
108+
)
109+
})
110+
const context = {
111+
subscriptions: [],
112+
extensionUri: {},
113+
} as unknown as ExtensionContext
114+
const { activate } = await import('./extension')
115+
116+
await activate(context)
117+
118+
expect(mocks.restart).toHaveBeenCalledOnce()
119+
expect(
120+
mocks.registerCommand.mock.calls.map(([command]) => command),
121+
).toEqual([
122+
'sqlmesh.signin',
123+
'sqlmesh.signinSpecifyFlow',
124+
'sqlmesh.signout',
125+
'sqlmesh.renderModel',
126+
'sqlmesh.showTableDiff',
127+
'sqlmesh.format',
128+
'sqlmesh.restart',
129+
'sqlmesh.stop',
130+
'sqlmesh.printEnvironment',
131+
])
132+
expect(mocks.registerContentProvider).toHaveBeenCalledOnce()
133+
expect(mocks.registerViewProvider).toHaveBeenCalledOnce()
134+
expect(
135+
context.subscriptions.filter(item => item === mocks.testController),
136+
).toHaveLength(1)
137+
},
138+
)
139+
140+
it('does not complete activation when both the initial start and retry fail', async () => {
141+
const failure = err({ type: 'sqlmesh_lsp_not_found' })
142+
mocks.start.mockImplementationOnce(() => {
143+
mocks.telemetryListener?.(false)
144+
return Promise.resolve(failure)
145+
})
146+
mocks.restart.mockResolvedValue(failure)
147+
const context = {
148+
subscriptions: [],
149+
extensionUri: {},
150+
} as unknown as ExtensionContext
151+
const { activate } = await import('./extension')
152+
153+
await activate(context)
154+
155+
expect(mocks.restart).toHaveBeenCalledOnce()
156+
expect(mocks.registerContentProvider).not.toHaveBeenCalled()
157+
expect(mocks.registerViewProvider).not.toHaveBeenCalled()
158+
expect(mocks.setupTestController).not.toHaveBeenCalled()
159+
})
160+
})

‎vscode/extension/src/extension.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ export async function activate(context: vscode.ExtensionContext) {
8383
* the run that is still waiting on the handler.
8484
*/
8585
let restartError: ErrorType | undefined
86+
// A retry during activation can succeed even when the first start failed.
87+
let lspStartSucceeded = false
8688

8789
const runRestart = async (): Promise<void> => {
8890
const invokedByUser = restartInvokedByUser
@@ -110,6 +112,7 @@ export async function activate(context: vscode.ExtensionContext) {
110112
}
111113
testControllerDisposable = setupTestController(lspClient)
112114
context.subscriptions.push(testControllerDisposable)
115+
lspStartSucceeded = true
113116
}
114117

115118
/**
@@ -158,7 +161,6 @@ export async function activate(context: vscode.ExtensionContext) {
158161
// telemetry change deferred forever.
159162
const initialLspClient = new LSPClient()
160163
lspClient = initialLspClient
161-
let initialStartSucceeded = false
162164
await telemetryRestartSubscription.runDuringInitialStart(async () => {
163165
const startResult = await initialLspClient.start()
164166
if (isErr(startResult)) {
@@ -175,12 +177,13 @@ export async function activate(context: vscode.ExtensionContext) {
175177

176178
// Initialize the test controller
177179
testControllerDisposable = setupTestController(initialLspClient)
178-
context.subscriptions.push(testControllerDisposable, testController)
179-
initialStartSucceeded = true
180+
context.subscriptions.push(testControllerDisposable)
181+
lspStartSucceeded = true
180182
})
181-
if (!initialStartSucceeded) {
183+
if (!lspStartSucceeded) {
182184
return // abort activation – nothing else to do
183185
}
186+
context.subscriptions.push(testController)
184187

185188
// Register the rendered model provider
186189
const renderedModelProvider = new RenderedModelProvider()

0 commit comments

Comments
 (0)