refactor: use URIs instead of paths (strings) in tests (#610)

### Summary of Changes

It's difficult to work with arbitrary paths, since they might be
relative to some other directory or absolute. This led to long variable
names to precisely express what a path meant. This PR instead refactors
the test creators and some helpers to provide URI objects instead.

---------

Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com>

tests/helpers/testChecks.test.ts

@@ -8,6 +8,7 @@ import {
     NoCommentsError,
 } from './testChecks.js';
 import { Range } from 'vscode-languageserver';
+import { URI } from 'langium';
 
 const uri = 'file:///test.sdstest';
 
@@ -75,7 +76,7 @@ ${OPEN}${CLOSE}
             id: 'two comments, two ranges',
         },
     ])('should associated comments and ranges ($id)', ({ program, expected }) => {
-        const result = findTestChecks(program, uri);
+        const result = findTestChecks(program, URI.parse(uri));
         expect(result.isOk).toBeTruthy();
 
         if (result.isOk) {
@@ -89,7 +90,7 @@ ${OPEN}${CLOSE}
             // $TEST$ no_syntax_error
             ${OPEN}\n${CLOSE}${CLOSE}
         `,
-            uri,
+            URI.parse(uri),
         );
         expect(result.isErr).toBeTruthy();
 
@@ -104,7 +105,7 @@ ${OPEN}${CLOSE}
             // $TEST$ no_syntax_error
             ${OPEN}\n${OPEN}${OPEN}${CLOSE}
         `,
-            uri,
+            URI.parse(uri),
         );
         expect(result.isErr).toBeTruthy();
 
@@ -119,7 +120,7 @@ ${OPEN}${CLOSE}
             // $TEST$ no_syntax_error
             ${OPEN}\n${CLOSE}${OPEN}\n${CLOSE}
         `,
-            uri,
+            URI.parse(uri),
         );
         expect(result.isErr).toBeTruthy();
 
@@ -129,7 +130,7 @@ ${OPEN}${CLOSE}
     });
 
     it('should report if no test comments are found if corresponding check is enabled', () => {
-        const result = findTestChecks('', uri, { failIfNoComments: true });
+        const result = findTestChecks('', URI.parse(uri), { failIfNoComments: true });
         expect(result.isErr).toBeTruthy();
 
         if (result.isErr) {
@@ -142,7 +143,7 @@ ${OPEN}${CLOSE}
             `
             // $TEST$ no_syntax_error
         `,
-            uri,
+            URI.parse(uri),
             { failIfFewerRangesThanComments: true },
         );
         expect(result.isErr).toBeTruthy();

tests/helpers/testChecks.ts

@@ -2,6 +2,7 @@ import { Location, Range } from 'vscode-languageserver';
 import { findTestComments } from './testComments.js';
 import { findTestRanges, FindTestRangesError } from './testRanges.js';
 import { Result } from 'true-myth';
+import { URI } from 'langium';
 
 /**
  * Finds all test checks, i.e. test comments and their corresponding test ranges.
@@ -12,7 +13,7 @@ import { Result } from 'true-myth';
  */
 export const findTestChecks = (
     program: string,
-    uri: string,
+    uri: URI,
     options: FindTestChecksOptions = {},
 ): Result<TestCheck[], FindTestChecksError> => {
     const { failIfNoComments = false, failIfFewerRangesThanComments = false } = options;
@@ -41,7 +42,12 @@ export const findTestChecks = (
         return Result.err(new FewerRangesThanCommentsError(comments, ranges));
     }
 
-    return Result.ok(comments.map((comment, index) => ({ comment, location: { uri, range: ranges[index] } })));
+    return Result.ok(
+        comments.map((comment, index) => ({
+            comment,
+            location: { uri: uri.toString(), range: ranges[index] },
+        })),
+    );
 };
 
 /**

tests/helpers/testResources.test.ts

@@ -1,9 +1,33 @@
 import { describe, expect, it } from 'vitest';
-import { listSafeDSResources, listTestsResourcesGroupedByParentDirectory } from './testResources.js';
+import {
+    listPythonFiles,
+    listSafeDsFiles,
+    listSafeDsFilesGroupedByParentDirectory,
+    ResourceName,
+    resourceNameToUri,
+    ShortenedResourceName,
+    uriToShortenedResourceName,
+} from './testResources.js';
+import { URI } from 'langium';
 
-describe('listTestResources', () => {
-    it('should yield all Safe-DS files in a directory that are not skipped', () => {
-        const result = listSafeDSResources('helpers/listTestResources');
+describe('uriToShortenedResourceName', () => {
+    it('should return the corresponding resource name if no root resource name is given', () => {
+        const resourceName = 'helpers/listSafeDsFiles';
+        const actual = uriToShortenedResourceName(resourceNameToUri(resourceName));
+        expect(normalizeResourceName(actual)).toBe(normalizeResourceName(resourceName));
+    });
+
+    it('should return a shortened resource name if a root resource name is given', () => {
+        const resourceName = 'helpers/nested/listSafeDsFiles';
+        const actual = uriToShortenedResourceName(resourceNameToUri(resourceName), 'helpers/nested');
+        expect(actual).toBe('listSafeDsFiles');
+    });
+});
+
+describe('listSafeDsFiles', () => {
+    it('should return all Safe-DS files in a resource directory that are not skipped', () => {
+        const rootResourceName = 'helpers/listSafeDsFiles';
+        const actual = listSafeDsFiles(rootResourceName);
         const expected = [
             'pipeline file.sdspipe',
             'stub file.sdsstub',
@@ -12,45 +36,85 @@ describe('listTestResources', () => {
             'nested/stub file.sdsstub',
             'nested/test file.sdstest',
         ];
-        expect(normalizePaths(result)).toStrictEqual(normalizePaths(expected));
+
+        expectFileListsToMatch(rootResourceName, actual, expected);
+    });
+});
+
+describe('listPythonFiles', () => {
+    it('should return all Python files in a resource directory', () => {
+        const rootResourceName = 'helpers/listPythonFiles';
+        const actual = listPythonFiles(rootResourceName);
+        const expected = ['python file.py', 'nested/python file.py'];
+
+        expectFileListsToMatch(rootResourceName, actual, expected);
     });
 });
 
-describe('listTestResourcesGroupedByParentDirectory', () => {
-    it('should yield all Safe-DS files in a directory that are not skipped and group them by parent directory', () => {
-        const result = listTestsResourcesGroupedByParentDirectory('helpers/listTestResources');
+describe('listSafeDsFilesGroupedByParentDirectory', () => {
+    it('should return all Safe-DS files in a directory that are not skipped and group them by parent directory', () => {
+        const rootResourceName = 'helpers/listSafeDsFiles';
+        const result = new Map(listSafeDsFilesGroupedByParentDirectory(rootResourceName));
 
-        const keys = Object.keys(result);
-        expect(normalizePaths(keys)).toStrictEqual(normalizePaths(['.', 'nested']));
+        // Compare the keys, i.e. the parent directories
+        const actualKeys = [...result.keys()];
+        const expectedKeys = ['', 'nested'];
+        expectFileListsToMatch(rootResourceName, actualKeys, expectedKeys);
 
-        const directlyInRoot = result['.'];
-        expect(normalizePaths(directlyInRoot)).toStrictEqual(
-            normalizePaths(['pipeline file.sdspipe', 'stub file.sdsstub', 'test file.sdstest']),
-        );
+        // Compare the values, i.e. the files, in the root directory
+        const actualValuesDirectlyInRoot = [...result.entries()].find(
+            ([key]) => uriToShortenedResourceName(key, rootResourceName) === '',
+        )!;
+        const expectedValuesDirectlyInRoot = ['pipeline file.sdspipe', 'stub file.sdsstub', 'test file.sdstest'];
+        expectFileListsToMatch(rootResourceName, actualValuesDirectlyInRoot[1], expectedValuesDirectlyInRoot);
 
-        const inNested = result.nested;
-        expect(normalizePaths(inNested)).toStrictEqual(
-            normalizePaths(['nested/pipeline file.sdspipe', 'nested/stub file.sdsstub', 'nested/test file.sdstest']),
-        );
+        // Compare the values, i.e. the files, in the nested directory
+        const actualValuesInNested = [...result.entries()].find(
+            ([key]) => uriToShortenedResourceName(key, rootResourceName) === 'nested',
+        )!;
+        const expectedValuesInNested = [
+            'nested/pipeline file.sdspipe',
+            'nested/stub file.sdsstub',
+            'nested/test file.sdstest',
+        ];
+        expectFileListsToMatch(rootResourceName, actualValuesInNested[1], expectedValuesInNested);
     });
 });
 
 /**
- * Normalizes the given paths by replacing backslashes with slashes and sorting them.
+ * Asserts that the actual uris and the expected shortened resource names point to the same files.
+ *
+ * @param rootResourceName The root resource name.
+ * @param actualUris The actual URIs computed by some function under test.
+ * @param expectedShortenedResourceNames The expected shortened resource names.
+ */
+const expectFileListsToMatch = (
+    rootResourceName: ResourceName,
+    actualUris: URI[],
+    expectedShortenedResourceNames: ShortenedResourceName[],
+): void => {
+    const actualShortenedResourceNames = actualUris.map((uri) => uriToShortenedResourceName(uri, rootResourceName));
+    expect(normalizeResourceNames(actualShortenedResourceNames)).toStrictEqual(
+        normalizeResourceNames(expectedShortenedResourceNames),
+    );
+};
+
+/**
+ * Normalizes the given resource names by replacing backslashes with slashes and sorting them.
  *
- * @param paths The paths to normalize.
- * @return The normalized paths.
+ * @param resourceNames The resource names to normalize.
+ * @return The normalized resource names.
  */
-const normalizePaths = (paths: string[]): string[] => {
-    return paths.map(normalizePath).sort();
+const normalizeResourceNames = (resourceNames: string[]): string[] => {
+    return resourceNames.map(normalizeResourceName).sort();
 };
 
 /**
- * Normalizes the given path by replacing backslashes with slashes.
+ * Normalizes the given resource name by replacing backslashes with slashes.
  *
- * @param path The path to normalize.
- * @return The normalized path.
+ * @param resourceName The resource name to normalize.
+ * @return The normalized resource name.
  */
-const normalizePath = (path: string): string => {
-    return path.replace(/\\/gu, '/');
+const normalizeResourceName = (resourceName: string): string => {
+    return resourceName.replace(/\\/gu, '/');
 };

tests/helpers/testResources.ts

@@ -2,57 +2,87 @@ import path from 'path';
 import { globSync } from 'glob';
 import { SAFE_DS_FILE_EXTENSIONS } from '../../src/language/helpers/fileExtensions.js';
 import { group } from 'radash';
+import { URI } from 'langium';
 
 const resourcesPath = path.join(__dirname, '..', 'resources');
 
 /**
- * Resolves the given path relative to `tests/resources/`.
+ * A path relative to `tests/resources/`.
+ */
+export type ResourceName = string;
+
+/**
+ * A path relative to `tests/resources/` or a subdirectory thereof.
+ */
+export type ShortenedResourceName = string;
+
+/**
+ * Returns the URI that corresponds to the resource with the given name.
  *
- * @param pathRelativeToResources The path relative to `tests/resources/`.
- * @return The resolved absolute path.
+ * @param resourceName The resource name.
+ * @return The corresponding URI.
  */
-export const resolvePathRelativeToResources = (pathRelativeToResources: string) => {
-    return path.join(resourcesPath, pathRelativeToResources);
+export const resourceNameToUri = (resourceName: ResourceName): URI => {
+    return URI.file(path.join(resourcesPath, resourceName));
 };
 
 /**
- * Lists all Safe-DS files in the given directory relative to `tests/resources/` that are not skipped.
+ * Returns the resource name that corresponds to the given URI. If `rootResourceName` is given, the result is relative
+ * to `tests/resources/<rootResourceName>`. Otherwise, the result is relative to `tests/resources/`.
  *
- * @param pathRelativeToResources The root directory relative to `tests/resources/`.
- * @return Paths to the Safe-DS files relative to `pathRelativeToResources`.
+ * @param uri The URI.
+ * @param rootResourceName The corresponding root resource name.
  */
-export const listSafeDSResources = (pathRelativeToResources: string): string[] => {
+export const uriToShortenedResourceName = (uri: URI, rootResourceName?: ResourceName): ShortenedResourceName => {
+    const rootPath = rootResourceName ? path.join(resourcesPath, rootResourceName) : resourcesPath;
+    return path.relative(rootPath, uri.fsPath);
+};
+
+/**
+ * Lists all Safe-DS files in the given root directory that are not skipped.
+ *
+ * @param rootResourceName The resource name of the root directory.
+ * @return URIs of the discovered Safe-DS files.
+ */
+export const listSafeDsFiles = (rootResourceName: ResourceName): URI[] => {
     const pattern = `**/*.{${SAFE_DS_FILE_EXTENSIONS.join(',')}}`;
-    const cwd = resolvePathRelativeToResources(pathRelativeToResources);
+    const cwd = resourceNameToUri(rootResourceName).fsPath;
 
-    return globSync(pattern, { cwd, nodir: true }).filter(isNotSkipped);
+    return globSync(pattern, { cwd, nodir: true })
+        .filter(isNotSkipped)
+        .map((it) => URI.file(path.join(cwd, it)));
 };
 
 /**
- * Lists all Python files in the given directory relative to `tests/resources/`.
+ * Lists all Python files in the given root directory.
  *
- * @param pathRelativeToResources The root directory relative to `tests/resources/`.
- * @return Paths to the Python files relative to `pathRelativeToResources`.
+ * @param rootResourceName The resource name of the root directory.
+ * @return URIs of the discovered Python files.
  */
-export const listPythonResources = (pathRelativeToResources: string): string[] => {
+export const listPythonFiles = (rootResourceName: ResourceName): URI[] => {
     const pattern = `**/*.py`;
-    const cwd = resolvePathRelativeToResources(pathRelativeToResources);
+    const cwd = resourceNameToUri(rootResourceName).fsPath;
 
-    return globSync(pattern, { cwd, nodir: true });
+    return globSync(pattern, { cwd, nodir: true }).map((it) => URI.file(path.join(cwd, it)));
 };
 
 /**
- * Lists all Safe-DS files in the given directory relative to `tests/resources/` that are not skipped. The result is
- * grouped by the parent directory.
+ * Lists all Safe-DS files in the given root directory that are not skipped. The result is grouped by the parent
+ * directory.
  *
- * @param pathRelativeToResources The root directory relative to `tests/resources/`.
- * @return Paths to the Safe-DS files relative to `pathRelativeToResources` grouped by the parent directory.
+ * @param rootResourceName The resource name of the root directory.
+ * @return URIs of the discovered Safe-DS files grouped by the parent directory.
  */
-export const listTestsResourcesGroupedByParentDirectory = (
-    pathRelativeToResources: string,
-): Record<string, string[]> => {
-    const paths = listSafeDSResources(pathRelativeToResources);
-    return group(paths, (p) => path.dirname(p)) as Record<string, string[]>;
+export const listSafeDsFilesGroupedByParentDirectory = (rootResourceName: ResourceName): [URI, URI[]][] => {
+    const uris = listSafeDsFiles(rootResourceName);
+    const groupedByParentDirectory = group(uris, (p) => path.dirname(p.fsPath)) as Record<string, URI[]>;
+
+    const result: [URI, URI[]][] = [];
+    for (const [parentDirectory, urisInParentDirectory] of Object.entries(groupedByParentDirectory)) {
+        result.push([URI.file(parentDirectory), urisInParentDirectory]);
+    }
+
+    return result;
 };
 
 const isNotSkipped = (pathRelativeToResources: string) => {