Skip to content

Commit 2cd1a6d

Browse files
mnoah1mnoah1
committed
Code review
1 parent 28f411f commit 2cd1a6d

File tree

2 files changed

+196
-6
lines changed

2 files changed

+196
-6
lines changed

‎src/test-explorer/resolver.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,22 @@ export class TestResolver implements OnModuleInit, vscode.Disposable {
8282
// Restore cached source files for targets that have them.
8383
const promises: Promise<void>[] = []
8484
for (const [key, testItem] of this.store.targetIdentifiers.entries()) {
85-
const target: bsp.BuildTargetIdentifier = JSON.parse(key)
86-
const sourceParams: bsp.SourcesParams = {
87-
targets: [target],
85+
let cachedSourceFiles: bsp.SourcesResult | undefined
86+
try {
87+
const target: bsp.BuildTargetIdentifier = JSON.parse(key)
88+
const sourceParams: bsp.SourcesParams = {
89+
targets: [target],
90+
}
91+
cachedSourceFiles = this.store.getCachedSourcesResult(sourceParams)
92+
} catch (e) {
93+
this.outputChannel.appendLine(
94+
`Invalid key '${key}' when restoring source files: ${e}`
95+
)
8896
}
89-
const cachedSourceFiles = this.store.getCachedSourcesResult(sourceParams)
97+
9098
if (cachedSourceFiles) {
9199
this.outputChannel.appendLine(
92-
`Restoring source files for target: ${target.uri}`
100+
`Restoring source files for target: ${key}`
93101
)
94102
try {
95103
testItem.canResolveChildren = false
@@ -100,7 +108,7 @@ export class TestResolver implements OnModuleInit, vscode.Disposable {
100108
promises.push(promise)
101109
} catch (e) {
102110
this.outputChannel.appendLine(
103-
`Error restoring source files for target: ${target.uri}`
111+
`Error restoring source files for target: ${key}`
104112
)
105113
}
106114
}

‎src/test/suite/resolver.test.ts‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,188 @@ suite('Test Resolver', () => {
907907
assert.equal(root.children.size, 0)
908908
assert.ok(root.description?.startsWith('Refresh Canceled:'))
909909
})
910+
911+
test('targets below root, restored', async () => {
912+
sendRequestStub.returns(Promise.resolve(sampleBuildTargetsResult))
913+
914+
workspaceStateGetStub
915+
.onFirstCall()
916+
.returns(sampleBuildTargetsResult)
917+
.onSecondCall()
918+
.returns(sampleSourceItemsResult(sampleBuildTargetsResult.targets[0]))
919+
920+
assert.ok(testCaseStore.testController.resolveHandler)
921+
922+
let resolveRegisterHandlers!: () => void
923+
const registerHandlersPromise = new Promise<void>(resolve => {
924+
resolveRegisterHandlers = resolve
925+
})
926+
927+
sandbox.stub(testResolver, 'registerHandlers').callsFake(() => {
928+
resolveRegisterHandlers()
929+
})
930+
931+
testResolver.onModuleInit()
932+
await registerHandlersPromise
933+
934+
// Validate items below the root
935+
const root = testCaseStore.testController.items.get('root')
936+
assert.ok(root)
937+
assert.equal(root.children.size, 2)
938+
validateIDValues(
939+
['e', '{targetdir}:/repo/root/base/directory'],
940+
root.children
941+
)
942+
943+
// Validate directory nesting
944+
const targetDirTestItem = root.children.get(
945+
'{targetdir}:/repo/root/base/directory'
946+
)
947+
assert.ok(targetDirTestItem)
948+
assert.equal(targetDirTestItem.children.size, 2)
949+
validateIDValues(
950+
[
951+
'{targetdir}:/repo/root/base/directory/a',
952+
'{targetdir}:/repo/root/base/directory/d',
953+
],
954+
targetDirTestItem.children
955+
)
956+
957+
// Validate targets placed under directories
958+
const sampleTarget = targetDirTestItem.children
959+
.get('{targetdir}:/repo/root/base/directory/d')
960+
?.children.get('d')
961+
assert.ok(sampleTarget)
962+
assert.ok(sampleTarget.canResolveChildren)
963+
assert.equal(sampleTarget.children.size, 0)
964+
965+
// Proper filtering based on target capabilities.
966+
const shouldBeExcluded = sampleBuildTargetsResult.targets.filter(
967+
target =>
968+
target.capabilities.canTest === false ||
969+
target.capabilities.canTest === undefined
970+
)
971+
972+
for (const excluded of shouldBeExcluded) {
973+
const recursiveCheck = (item: vscode.TestItem) => {
974+
item.children.forEach(child => {
975+
recursiveCheck(child)
976+
assert.equal(item.children.get(excluded.id.uri), undefined)
977+
})
978+
}
979+
recursiveCheck(root)
980+
}
981+
})
982+
983+
test('expand targets based on open files, restored', async () => {
984+
const sourcesResultCache: Record<string, bsp.SourcesResult> = {}
985+
for (const target of sampleBuildTargetsResult.targets) {
986+
sourcesResultCache[
987+
JSON.stringify({
988+
targets: [target.id],
989+
})
990+
] = sampleSourceItemsResult(target)
991+
}
992+
993+
// JSON parse failure for one target
994+
const jsonParseStub = sandbox.stub(JSON, 'parse')
995+
jsonParseStub.onFirstCall().callsFake(() => {
996+
jsonParseStub.restore()
997+
throw new Error('json parse error')
998+
})
999+
1000+
workspaceStateGetStub
1001+
.onFirstCall()
1002+
.returns(sampleBuildTargetsResult)
1003+
.onSecondCall()
1004+
.returns(sourcesResultCache)
1005+
1006+
languageToolsStub.getDocumentTestCases.resolves({
1007+
isTestFile: true,
1008+
testCases: getSampleDocumentTestItems(),
1009+
documentTest: {
1010+
name: 'My Document',
1011+
range: new vscode.Range(
1012+
new vscode.Position(0, 0),
1013+
new vscode.Position(0, 0)
1014+
),
1015+
uri: documentStub.uri,
1016+
testFilter: 'test_file.py',
1017+
},
1018+
})
1019+
1020+
let resolveRegisterHandlers!: () => void
1021+
const registerHandlersPromise = new Promise<void>(resolve => {
1022+
resolveRegisterHandlers = resolve
1023+
})
1024+
1025+
sandbox.stub(testResolver, 'registerHandlers').callsFake(() => {
1026+
resolveRegisterHandlers()
1027+
})
1028+
1029+
testResolver.onModuleInit()
1030+
await registerHandlersPromise
1031+
const root = testCaseStore.testController.items.get('root')
1032+
assert.ok(root)
1033+
1034+
// Validate expected document positions in the tree.
1035+
const docTestItem = [
1036+
// Target a's files not reloaded successfully.
1037+
root.children
1038+
.get('{targetdir}:/repo/root/base/directory')
1039+
?.children.get('{targetdir}:/repo/root/base/directory/a')
1040+
?.children.get('a'),
1041+
// Target's d's files reloaded successfully.
1042+
root.children
1043+
.get('{targetdir}:/repo/root/base/directory')
1044+
?.children.get('{targetdir}:/repo/root/base/directory/d')
1045+
?.children.get('d')
1046+
?.children.get(
1047+
'{sourcefile}:d:/repo/root/base/directory/d/MyFile1.language'
1048+
),
1049+
root.children
1050+
.get('{targetdir}:/repo/root/base/directory')
1051+
?.children.get('{targetdir}:/repo/root/base/directory/d')
1052+
?.children.get('d')
1053+
?.children.get(
1054+
'{sourcefile}:d:/repo/root/base/directory/d/MyFile2.language'
1055+
),
1056+
root.children
1057+
.get('{targetdir}:/repo/root/base/directory')
1058+
?.children.get('{targetdir}:/repo/root/base/directory/d')
1059+
?.children.get('d')
1060+
?.children.get('{sourcedir}:d:/repo/root/base/directory/d/src/dir')
1061+
?.children.get('{sourcedir}:d:/repo/root/base/directory/d/src/dir/1')
1062+
?.children.get(
1063+
'{sourcefile}:d:/repo/root/base/directory/d/src/dir/1/MyFile4.language'
1064+
),
1065+
root.children
1066+
.get('{targetdir}:/repo/root/base/directory')
1067+
?.children.get('{targetdir}:/repo/root/base/directory/d')
1068+
?.children.get('d')
1069+
?.children.get('{sourcedir}:d:/repo/root/base/directory/d/src/dir')
1070+
?.children.get('{sourcedir}:d:/repo/root/base/directory/d/src/dir/2')
1071+
?.children.get(
1072+
'{sourcefile}:d:/repo/root/base/directory/d/src/dir/2/MyFile5.language'
1073+
),
1074+
root.children
1075+
.get('{targetdir}:/repo/root/base/directory')
1076+
?.children.get('{targetdir}:/repo/root/base/directory/d')
1077+
?.children.get('d')
1078+
?.children.get('{sourcedir}:d:/repo/root/base/directory/d/src/dir')
1079+
?.children.get('{sourcedir}:d:/repo/root/base/directory/d/src/dir/2')
1080+
?.children.get(
1081+
'{sourcefile}:d:/repo/root/base/directory/d/src/dir/2/MyFile6.language'
1082+
),
1083+
]
1084+
1085+
for (const item of docTestItem) {
1086+
assert.ok(item)
1087+
item.children.forEach(child => {
1088+
assert.ok(testCaseStore.testCaseMetadata.get(child))
1089+
})
1090+
}
1091+
})
9101092
})
9111093
})
9121094

0 commit comments

Comments
 (0)