Skip to content

Commit 513e2a7

Browse files
fix: Make tests more robust & fix path bug
1 parent a29134b commit 513e2a7

File tree

10 files changed

+629
-76
lines changed

10 files changed

+629
-76
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ yarn-debug.log*
66
yarn-error.log*
77
lerna-debug.log*
88

9+
# Editor files
10+
.idea/
11+
912
# Diagnostic reports (https://nodejs.org/api/report.html)
1013
report.[0-9]*.[0-9]*.[0-9]*.[0-9]*.json
1114

packages/pyright-internal/src/analyzer/program.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,15 @@ export class Program {
331331

332332
addTrackedFile(filePath: string, isThirdPartyImport = false, isInPyTypedPackage = false): SourceFile {
333333
let sourceFileInfo = this.getSourceFileInfo(filePath);
334-
const importName = this._getImportNameForFile(filePath);
334+
let importName = this._getImportNameForFile(filePath);
335+
// HACK(scip-python): When adding tracked files for imports, we end up passing
336+
// normalized paths as the argument. However, _getImportNameForFile seemingly
337+
// needs a non-normalized path, which cannot be recovered directly from a
338+
// normalized path. However, in practice, the non-normalized path seems to
339+
// be stored on the sourceFileInfo, so attempt to use that instead.
340+
if (importName === '' && sourceFileInfo) {
341+
importName = this._getImportNameForFile(sourceFileInfo.sourceFile.getFilePath());
342+
}
335343

336344
if (sourceFileInfo) {
337345
// The module name may have changed based on updates to the

packages/pyright-scip/CONTRIBUTING.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,32 @@ node ./index.js <other args>
7373
npm run check-snapshots
7474
```
7575

76+
#### Filter specific snapshot tests
77+
78+
Use the `--filter-tests` flag to run only specific snapshot tests:
79+
```bash
80+
# Using npm scripts (note the -- to pass arguments)
81+
npm run check-snapshots -- --filter-tests test1,test2,test3
82+
```
83+
84+
Available snapshot tests can be found in `snapshots/input/`.
85+
7686
Using a different Python version other than the one specified
7787
in `.tool-versions` may also lead to errors.
7888

89+
## Making changes to Pyright internals
90+
91+
When modifying code in the `pyright-internal` package:
92+
93+
1. Keep changes minimal: Every change introduces a risk of
94+
merge conflicts. Adding doc comments is fine, but avoid
95+
changing functionality if possible. Instead of changing
96+
access modifiers, prefer copying small functions into
97+
scip-pyright logic.
98+
2. Use a `NOTE(scip-python):` prefix when adding comments to
99+
make it clearer which comments were added by upstream
100+
maintainers vs us.
101+
79102
## Publishing releases
80103

81104
1. Change the version in `packages/pyright-scip/package.json`
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
# < definition scip-python python snapshot-util 0.1 `src.long_importer`/__init__:
22

33
import foo.bar.baz.mod
4-
# ^^^^^^^^^^^^^^^ reference snapshot-util 0.1 `foo.bar.baz.mod`/__init__:
4+
# ^^^^^^^^^^^^^^^ reference local 0
55

66
print(foo.bar.baz.mod.SuchNestedMuchWow)
77
#^^^^ reference python-stdlib 3.11 builtins/print().
8-
# ^^^^^^^^^^^^^^^ reference snapshot-util 0.1 `foo.bar.baz.mod`/__init__:
8+
# ^^^ reference local 0
99
# ^^^^^^^^^^^^^^^^^ reference snapshot-util 0.1 `src.foo.bar.baz.mod`/SuchNestedMuchWow#
1010

packages/pyright-scip/src/indexer.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ export class Indexer {
123123
this.importResolver = new ImportResolver(fs, this.pyrightConfig, host);
124124

125125
this.program = new Program(this.importResolver, this.pyrightConfig);
126-
// Normalize paths to ensure consistency with other code paths.
127-
const normalizedProjectFiles = [...this.projectFiles].map((path: string) => normalizePathCase(fs, path));
128-
this.program.setTrackedFiles(normalizedProjectFiles);
126+
// setTrackedFiles internally handles path normalization, so we don't normalize
127+
// paths here.
128+
this.program.setTrackedFiles([...this.projectFiles]);
129129

130130
if (scipConfig.projectNamespace) {
131131
setProjectNamespace(scipConfig.projectName, this.scipConfig.projectNamespace!);
@@ -194,7 +194,9 @@ export class Indexer {
194194
let projectSourceFiles: SourceFile[] = [];
195195
withStatus('Index workspace and track project files', () => {
196196
this.program.indexWorkspace((filepath: string) => {
197-
// Filter out filepaths not part of this project
197+
// Do not index files outside the project because SCIP doesn't support it.
198+
//
199+
// Both filepath and this.scipConfig.projectRoot are NOT normalized.
198200
if (filepath.indexOf(this.scipConfig.projectRoot) != 0) {
199201
return;
200202
}
@@ -204,6 +206,7 @@ export class Indexer {
204206

205207
let requestsImport = sourceFile.getImports();
206208
requestsImport.forEach((entry) =>
209+
// entry.resolvedPaths are all normalized.
207210
entry.resolvedPaths.forEach((value) => {
208211
this.program.addTrackedFile(value, true, false);
209212
})
@@ -226,6 +229,7 @@ export class Indexer {
226229
progress.progress(`(${index}/${projectSourceFiles.length}): ${sourceFile.getFilePath()}`);
227230

228231
const filepath = sourceFile.getFilePath();
232+
229233
let doc = new scip.Document({
230234
relative_path: path.relative(this.getProjectRoot(), filepath),
231235
});

packages/pyright-scip/src/lib.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,13 +301,32 @@ export function formatSnapshot(
301301
return out.join('');
302302
}
303303

304-
export function writeSnapshot(outputPath: string, obtained: string): void {
304+
export function writeSnapshot(outputPath: string, obtained: string, tempDir?: string): void {
305+
let finalPath: string;
306+
let targetDir: string;
307+
308+
if (tempDir) {
309+
// Extract the relative path from the test output directory to preserve structure
310+
const outputParts = outputPath.split(path.sep);
311+
const testOutputIndex = outputParts.findIndex(part => part === 'output') + 1;
312+
if (testOutputIndex > 0 && testOutputIndex < outputParts.length) {
313+
const relativeStructure = outputParts.slice(testOutputIndex + 1).join(path.sep);
314+
finalPath = path.join(tempDir, relativeStructure);
315+
} else {
316+
finalPath = path.join(tempDir, path.basename(outputPath));
317+
}
318+
targetDir = path.dirname(finalPath);
319+
} else {
320+
finalPath = outputPath;
321+
targetDir = path.dirname(outputPath);
322+
}
323+
305324
// eslint-disable-next-line no-sync
306-
fs.mkdirSync(path.dirname(outputPath), {
325+
fs.mkdirSync(targetDir, {
307326
recursive: true,
308327
});
309328
// eslint-disable-next-line no-sync
310-
fs.writeFileSync(outputPath, obtained, { flag: 'w' });
329+
fs.writeFileSync(finalPath, obtained, { flag: 'w' });
311330
}
312331

313332
export function diffSnapshot(outputPath: string, obtained: string): void {
@@ -329,6 +348,19 @@ export function diffSnapshot(outputPath: string, obtained: string): void {
329348
exit(1);
330349
}
331350

351+
export function checkSnapshotMatch(outputPath: string, obtained: string): boolean {
352+
try {
353+
if (!fs.existsSync(outputPath)) {
354+
return false;
355+
}
356+
357+
const existing = fs.readFileSync(outputPath, { encoding: 'utf8' });
358+
return obtained === existing;
359+
} catch (error) {
360+
return false;
361+
}
362+
}
363+
332364
function occurrencesByLine(a: scip.Occurrence, b: scip.Occurrence): number {
333365
return Range.fromLsif(a.range).compare(Range.fromLsif(b.range));
334366
}

0 commit comments

Comments
 (0)