Skip to content

Commit 1d14d39

Browse files
committed
Make the background job testable
1 parent 0e51426 commit 1d14d39

File tree

4 files changed

+82
-77
lines changed

4 files changed

+82
-77
lines changed

server/src/__tests__/analyzer.test.ts

Lines changed: 17 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,14 @@ import * as fsUtil from '../util/fs'
77
let analyzer: Analyzer
88

99
const CURRENT_URI = 'dummy-uri.sh'
10+
const mockConsole = getMockConnection().console
11+
12+
// if you add a .sh file to testing/fixtures, update this value
13+
const FIXTURE_FILES_MATCHING_GLOB = 13
1014

1115
beforeAll(async () => {
1216
const parser = await initializeParser()
13-
analyzer = new Analyzer(parser)
17+
analyzer = new Analyzer({ console: mockConsole, parser })
1418
})
1519

1620
describe('analyze', () => {
@@ -262,39 +266,28 @@ describe('commentsAbove', () => {
262266
})
263267
})
264268

265-
describe('fromRoot', () => {
266-
it('initializes an analyzer from a root', async () => {
269+
describe('initiateBackgroundAnalysis', () => {
270+
it('finds bash files', async () => {
267271
const parser = await initializeParser()
268272

269273
jest.spyOn(Date, 'now').mockImplementation(() => 0)
270274

271275
const connection = getMockConnection()
272276

273-
const newAnalyzer = await Analyzer.fromRoot({
274-
connection,
277+
const newAnalyzer = new Analyzer({ console: connection.console, parser })
278+
const { filesParsed } = await newAnalyzer.initiateBackgroundAnalysis({
275279
rootPath: FIXTURE_FOLDER,
276-
parser,
277280
})
278281

279-
expect(newAnalyzer).toBeDefined()
280-
281282
expect(connection.window.showWarningMessage).not.toHaveBeenCalled()
282-
283-
// if you add a .sh file to testing/fixtures, update this value
284-
const FIXTURE_FILES_MATCHING_GLOB = 14
283+
expect(connection.console.warn).not.toHaveBeenCalled()
285284

286285
// Intro, stats on glob, one file skipped due to shebang, and outro
287-
const LOG_LINES = FIXTURE_FILES_MATCHING_GLOB + 4
286+
expect(filesParsed).toEqual(FIXTURE_FILES_MATCHING_GLOB)
288287

289-
expect(connection.console.log).toHaveBeenCalledTimes(LOG_LINES)
290288
expect(connection.console.log).toHaveBeenNthCalledWith(
291289
1,
292-
expect.stringContaining('Analyzing files matching'),
293-
)
294-
295-
expect(connection.console.log).toHaveBeenNthCalledWith(
296-
LOG_LINES,
297-
'Analyzer finished after 0 seconds',
290+
expect.stringContaining('BackgroundAnalysis: resolving glob'),
298291
)
299292
})
300293

@@ -307,16 +300,13 @@ describe('fromRoot', () => {
307300

308301
const connection = getMockConnection()
309302

310-
const newAnalyzer = await Analyzer.fromRoot({
311-
connection,
303+
const newAnalyzer = new Analyzer({ console: connection.console, parser })
304+
const { filesParsed } = await newAnalyzer.initiateBackgroundAnalysis({
312305
rootPath: FIXTURE_FOLDER,
313-
parser,
314306
})
315307

316-
expect(newAnalyzer).toBeDefined()
317-
318-
expect(connection.window.showWarningMessage).toHaveBeenCalledWith(
319-
expect.stringContaining('BOOM'),
320-
)
308+
expect(connection.window.showWarningMessage).not.toHaveBeenCalled()
309+
expect(connection.console.warn).toHaveBeenCalledWith(expect.stringContaining('BOOM'))
310+
expect(filesParsed).toEqual(0)
321311
})
322312
})

server/src/__tests__/server.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,12 @@ async function initializeServer() {
2020
workspaceFolders: null,
2121
})
2222

23+
const { backgroundAnalysisCompleted } = server
24+
25+
if (backgroundAnalysisCompleted) {
26+
await backgroundAnalysisCompleted
27+
}
28+
2329
return {
2430
connection,
2531
console,

server/src/analyser.ts

Lines changed: 25 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -28,34 +28,8 @@ type Texts = { [uri: string]: string }
2828
* tree-sitter to find definitions, reference, etc.
2929
*/
3030
export default class Analyzer {
31-
/**
32-
* Initialize the Analyzer based on a connection to the client and an optional
33-
* root path.
34-
*
35-
* If the rootPath is provided it will initialize all shell files it can find
36-
* anywhere on that path. This non-exhaustive glob is used to preload the parser
37-
* to support features across files.
38-
*/
39-
public static fromRoot({
40-
connection,
41-
rootPath,
42-
parser,
43-
}: {
44-
connection: LSP.Connection
45-
rootPath: LSP.InitializeParams['rootPath']
46-
parser: Parser
47-
}): Analyzer {
48-
const analyzer = new Analyzer(parser)
49-
50-
if (rootPath) {
51-
// NOTE: we do not block the initialization on this background analysis.
52-
analyzer.initiateBackgroundAnalysis({ connection, rootPath })
53-
}
54-
55-
return analyzer
56-
}
57-
5831
private parser: Parser
32+
private console: LSP.RemoteConsole
5933

6034
private uriToTextDocument: { [uri: string]: LSP.TextDocument } = {}
6135

@@ -73,20 +47,29 @@ export default class Analyzer {
7347
variable_assignment: LSP.SymbolKind.Variable,
7448
}
7549

76-
public constructor(parser: Parser) {
50+
public constructor({
51+
console,
52+
parser,
53+
}: {
54+
console: LSP.RemoteConsole
55+
parser: Parser
56+
}) {
7757
this.parser = parser
58+
this.console = console
7859
}
7960

80-
private async initiateBackgroundAnalysis({
81-
connection,
61+
/**
62+
* Initiates a background analysis of the files in the given rootPath to
63+
* enable features across files.
64+
*/
65+
public async initiateBackgroundAnalysis({
8266
rootPath,
8367
}: {
84-
connection: LSP.Connection
8568
rootPath: string
86-
}) {
69+
}): Promise<{ filesParsed: number }> {
8770
const globPattern = config.getGlobPattern()
8871
const backgroundAnalysisMaxFiles = config.getBackgroundAnalysisMaxFiles()
89-
connection.console.log(
72+
this.console.log(
9073
`BackgroundAnalysis: resolving glob "${globPattern}" inside "${rootPath}"...`,
9174
)
9275

@@ -102,13 +85,13 @@ export default class Analyzer {
10285
})
10386
} catch (error) {
10487
const errorMessage = error instanceof Error ? error.message : error
105-
connection.window.showWarningMessage(
88+
this.console.warn(
10689
`BackgroundAnalysis: failed resolved glob "${globPattern}". The experience across files will be degraded. Error: ${errorMessage}`,
10790
)
108-
return
91+
return { filesParsed: 0 }
10992
}
11093

111-
connection.console.log(
94+
this.console.log(
11295
`BackgroundAnalysis: Glob resolved with ${
11396
filePaths.length
11497
} files after ${getTimePassed()}`,
@@ -121,7 +104,7 @@ export default class Analyzer {
121104
const fileContent = await readFileAsync(filePath, 'utf8')
122105
const shebang = getShebang(fileContent)
123106
if (shebang && !isBashShebang(shebang)) {
124-
connection.console.log(
107+
this.console.log(
125108
`BackgroundAnalysis: Skipping file ${uri} with shebang "${shebang}"`,
126109
)
127110
continue
@@ -130,13 +113,16 @@ export default class Analyzer {
130113
this.analyze(uri, LSP.TextDocument.create(uri, 'shell', 1, fileContent))
131114
} catch (error) {
132115
const errorMessage = error instanceof Error ? error.message : error
133-
connection.console.warn(
116+
this.console.warn(
134117
`BackgroundAnalysis: Failed analyzing ${uri}. Error: ${errorMessage}`,
135118
)
136119
}
137120
}
138121

139-
connection.console.log(`BackgroundAnalysis: Completed after ${getTimePassed()}.`)
122+
this.console.log(`BackgroundAnalysis: Completed after ${getTimePassed()}.`)
123+
return {
124+
filesParsed: filePaths.length,
125+
}
140126
}
141127

142128
/**

server/src/server.ts

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,25 @@ export default class BashServer {
3838
throw new Error('Expected PATH environment variable to be set')
3939
}
4040

41-
const analyzer = Analyzer.fromRoot({ connection, rootPath, parser })
42-
4341
return Promise.all([Executables.fromPath(PATH), getLinterExecutablePath()]).then(
4442
([executables, linterExecutablePath]) => {
43+
const analyzer = new Analyzer({ console: connection.console, parser })
4544
const linter = new Linter({ executablePath: linterExecutablePath })
46-
return new BashServer(connection, executables, analyzer, linter, capabilities)
45+
46+
let backgroundAnalysisCompleted
47+
if (rootPath) {
48+
// NOTE: we do not block the server initialization on this background analysis.
49+
backgroundAnalysisCompleted = analyzer.initiateBackgroundAnalysis({ rootPath })
50+
}
51+
52+
return new BashServer({
53+
analyzer,
54+
backgroundAnalysisCompleted,
55+
capabilities,
56+
connection,
57+
executables,
58+
linter,
59+
})
4760
},
4861
)
4962
}
@@ -55,19 +68,29 @@ export default class BashServer {
5568
private documents: LSP.TextDocuments<TextDocument> = new LSP.TextDocuments(TextDocument)
5669
private connection: LSP.Connection
5770
private clientCapabilities: LSP.ClientCapabilities
58-
59-
private constructor(
60-
connection: LSP.Connection,
61-
executables: Executables,
62-
analyzer: Analyzer,
63-
linter: Linter,
64-
capabilities: LSP.ClientCapabilities,
65-
) {
71+
public backgroundAnalysisCompleted?: Promise<any>
72+
73+
private constructor({
74+
connection,
75+
executables,
76+
analyzer,
77+
linter,
78+
capabilities,
79+
backgroundAnalysisCompleted,
80+
}: {
81+
connection: LSP.Connection
82+
executables: Executables
83+
analyzer: Analyzer
84+
linter: Linter
85+
capabilities: LSP.ClientCapabilities
86+
backgroundAnalysisCompleted?: Promise<any>
87+
}) {
6688
this.connection = connection
6789
this.executables = executables
6890
this.analyzer = analyzer
6991
this.linter = linter
7092
this.clientCapabilities = capabilities
93+
this.backgroundAnalysisCompleted = backgroundAnalysisCompleted
7194
}
7295

7396
/**

0 commit comments

Comments
 (0)