Skip to content

Commit 3f64e25

Browse files
test: isolated integration tests
Previously: 1. Integration tests were run in a subfolder within the root project, allowing them to inherit missing dependencies from the root. 2. `npm/yarn link` was used to integrate Sheriff, resulting in Sheriff using the TypeScript version from the root project instead of the version within the integration tests. These issues prevented the tests from detecting missing dependencies, such as the "typescript-eslint" package (as reported in #114), and caused failures due to version mismatches in TypeScript. **Changes:** - Integration tests have been moved to a temporary directory outside the root project, ensuring complete isolation. - `yalc` is now used instead of `npm/yarn link`, eliminating symlinks and enforcing the use of the correct dependencies.
1 parent c45c4fd commit 3f64e25

File tree

17 files changed

+722
-90
lines changed

17 files changed

+722
-90
lines changed

.github/workflows/build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ jobs:
1515
with:
1616
node-version: '18'
1717
cache: 'yarn'
18-
- run: npm install -g yarn
18+
- run: npm install -g yarn yalc
1919
- run: yarn
2020
- run: yarn lint:all
2121
- run: yarn link:sheriff

.gitignore

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,7 @@ testem.log
3838
.DS_Store
3939
Thumbs.db
4040

41-
.nx/cache
41+
.nx
42+
.yalc
43+
yalc.lock
44+
.test-projects

DEVELOPMENT.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Sheriff consists of three processes:
2+
3+
1. File Graph: `traverseFilesystem` gets and entry file and returns a graph of all the files that are required to run
4+
the entry file. The final type of the graph is `UnassignedFileInfo`, meaning graph without modules.
5+
2. Modules: Based on `FileInfo`, `createModules` detects the existing modules and their dependencies. The final type of
6+
the modules is `ModuleInfo`.
7+
3. Merging FileGraph and Modules: `FileInfo` is the final type of the graph that contains all the information about the
8+
files and modules. It is done by
9+
10+
The entry point is always the `init` function.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"build:all": "npx nx run-many --target=build && chmod +x dist/packages/core/src/bin/main.js",
88
"commit": "commit",
99
"lint:all": "eslint ./packages",
10-
"link:sheriff": "npm run build:all && yarn link --cwd dist/packages/core && yarn link --cwd dist/packages/eslint-plugin && yarn link @softarc/sheriff-core @softarc/eslint-plugin-sheriff",
10+
"link:sheriff": "npm run build:all && yalc publish dist/packages/core && yalc publish dist/packages/eslint-plugin && yalc link @softarc/sheriff-core @softarc/eslint-plugin-sheriff",
1111
"run:cli": "nx build core && chmod +x dist/packages/core/src/bin/main.js",
1212
"test": "vitest",
1313
"test:ci": "vitest -c vitest.config.ci.ts"

packages/core/src/lib/file-info/traverse-filesystem.ts

Lines changed: 65 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import UnassignedFileInfo from './unassigned-file-info';
22
import getFs from '../fs/getFs';
33
import * as ts from 'typescript';
44
import { TsData, TsPaths } from './ts-data';
5-
import { FsPath, toFsPath } from './fs-path';
5+
import { FsPath, isFsPath, toFsPath } from './fs-path';
66

77
export type ResolveFn = (
88
moduleName: string,
@@ -21,6 +21,9 @@ export type ResolveFn = (
2121
* filesystem happens. In case the abstraction does not emulate the original's
2222
* behaviour, "strange bugs" might occur. Look out for them.
2323
*
24+
* fixPathSeparators is necessary to replace the static '/' path separator
25+
* with the one from the OS.
26+
*
2427
* @param fsPath Filename to traverse from
2528
* @param fileInfoDict Dictionary of traversed files to catch circularity
2629
* @param tsData
@@ -57,14 +60,21 @@ const traverseFilesystem = (
5760
throw new Error(`${importPath} is outside of root ${rootDir}`);
5861
}
5962
}
60-
} else if (fileName.endsWith('.json')) {
61-
// just skip it
62-
} else if (fileName.startsWith('.')) {
63-
// might be an undetected dependency in node_modules
64-
// or an incomplete import (= developer is still typing),
65-
// if we read from an unsaved file via ESLint.
63+
}
64+
65+
// just skip it
66+
else if (fileName.endsWith('.json')) {
67+
}
68+
69+
// might be an undetected dependency in node_modules
70+
// or an incomplete import (= developer is still typing),
71+
// if we read from an unsaved file via ESLint.
72+
else if (fileName.startsWith('.')) {
6673
fileInfo.addUnresolvableImport(fileName);
67-
} else {
74+
}
75+
76+
// check for path/alias mapping
77+
else {
6878
const resolvedTsPath = resolvePotentialTsPath(
6979
fileName,
7080
paths,
@@ -103,27 +113,36 @@ export function resolvePotentialTsPath(
103113
let unpathedImport: string | undefined;
104114
for (const tsPath in tsPaths) {
105115
const { isWildcard, clearedTsPath } = clearTsPath(tsPath);
116+
// import from '@app/app.component' & paths: {'@app/*': ['src/app/*']}
106117
if (isWildcard && moduleName.startsWith(clearedTsPath)) {
107118
const pathMapping = tsPaths[tsPath];
108119
unpathedImport = moduleName.replace(clearedTsPath, pathMapping);
109-
} else if (tsPath === moduleName) {
120+
}
121+
// import from '@app' & paths: { '@app': [''] }
122+
else if (tsPath === moduleName) {
110123
unpathedImport = tsPaths[tsPath];
111124
}
112125

113126
if (unpathedImport) {
114-
const resolvedImport = resolveFn(unpathedImport);
115-
116-
if (
117-
!resolvedImport.resolvedModule ||
118-
resolvedImport.resolvedModule.isExternalLibraryImport === true
119-
) {
120-
throw new Error(
121-
`unable to resolve import ${moduleName} in ${filename}`,
127+
// path is file -> return as is
128+
if (isPathFile(unpathedImport)) {
129+
return fixPathSeparators(toFsPath(unpathedImport));
130+
}
131+
// path is directory or something else -> rely on TypeScript resolvers
132+
else {
133+
const resolvedImport = resolveFn(unpathedImport);
134+
if (
135+
!resolvedImport.resolvedModule ||
136+
resolvedImport.resolvedModule.isExternalLibraryImport === true
137+
) {
138+
throw new Error(
139+
`unable to resolve import ${moduleName} in ${filename}`,
140+
);
141+
}
142+
return toFsPath(
143+
fixPathSeparators(resolvedImport.resolvedModule.resolvedFileName),
122144
);
123145
}
124-
return toFsPath(
125-
fixPathSeparators(resolvedImport.resolvedModule.resolvedFileName),
126-
);
127146
}
128147
}
129148

@@ -137,6 +156,12 @@ function clearTsPath(tsPath: string) {
137156
return { isWildcard, clearedTsPath: clearedPath };
138157
}
139158

159+
160+
/**
161+
* Ensures that `FsPath` uses the separator from the OS and not always '/'
162+
*
163+
* @param path
164+
*/
140165
function fixPathSeparators(path: string): FsPath {
141166
const fs = getFs();
142167

@@ -147,4 +172,24 @@ function fixPathSeparators(path: string): FsPath {
147172
return toFsPath(path);
148173
}
149174

175+
/**
176+
* Checks if the path is a file.
177+
*
178+
* For example in tsconfig.json:
179+
* ```json
180+
* "paths": {
181+
* "@app": ["src/app/index"]
182+
* }
183+
* ```
184+
*
185+
* 'src/app/index' comes already as absolute path with ts extension from
186+
* pre-processing of @generateTsData.
187+
*
188+
* @param path '/.../src/app/index.ts' according to the example above
189+
*/
190+
function isPathFile(path: string): boolean {
191+
const fs = getFs();
192+
return fs.exists(path) && isFsPath(path) && fs.isFile(path);
193+
}
194+
150195
export default traverseFilesystem;

packages/core/src/lib/fs/default-fs.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ export class DefaultFs extends Fs {
8989
override split = (p: string) => p.split(path.sep);
9090

9191
print = () => void true;
92+
93+
override isFile(path: FsPath): boolean {
94+
return fs.lstatSync(path).isFile();
95+
}
9296
}
9397

9498
const defaultFs = new DefaultFs();

packages/core/src/lib/fs/fs.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,6 @@ export abstract class Fs {
4545
abstract split(path: string): string[];
4646

4747
abstract isAbsolute(path: string): boolean;
48+
49+
abstract isFile(path: FsPath): boolean
4850
}

packages/core/src/lib/fs/virtual-fs.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,11 @@ export class VirtualFs extends Fs {
286286
override split(path: string): string[] {
287287
return path.split('/');
288288
}
289+
290+
override isFile(path: FsPath): boolean {
291+
const node = this.#getNodeOrThrow(path);
292+
return node.node.type === 'file'
293+
}
289294
}
290295

291296
const virtualFs = new VirtualFs();

run-integration-tests.sh

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,24 @@
11
set -e
22

3-
# This is necessary because @angular-eslint uses nx and would
4-
# use root as workspace... ending up in strange error messages.
5-
# This can go away as soon as angular-eslint switches to flat
6-
# config.
3+
# We copy the test projects to a temporary directory to avoid any potential
4+
# issues with the dependencies from the root project.
75

8-
mv nx.json nx.json.bak
6+
# Check if .test-projects exists and is a symbolic link
7+
if [ -L .test-projects ]; then
8+
TARGET_DIR=$(readlink .test-projects)
9+
rm .test-projects
10+
rm -rf "$TARGET_DIR"
11+
echo "Removing $TARGET_DIR"
12+
fi
13+
14+
export TMP_DIR=$(mktemp -d)
15+
rsync -a --exclude node_modules --exclude .angular test-projects/ "$TMP_DIR"
16+
ln -sf "$TMP_DIR" .test-projects
17+
18+
echo "Temporary directory created at $TMP_DIR"
919

1020
echo "Testing against Angular 15 (ESLint Legacy)"
11-
cd test-projects/angular-i
21+
cd .test-projects/angular-i
1222
bash ./integration-test.sh
1323

1424
echo "Testing against Angular 18 (ESLint Flat)"
@@ -19,4 +29,5 @@ cd ../typescript-i
1929
bash ./integration-test.sh
2030

2131
cd ../..
22-
mv nx.json.bak nx.json
32+
33+
echo "Tests finished successfully"
Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,24 @@
11
set -e
22
yarn
3+
yalc add @softarc/sheriff-core @softarc/eslint-plugin-sheriff
4+
cd node_modules/.bin # yalc doesn't create symlink in node_modules/.bin
5+
ln -s ../@softarc/sheriff-core/src/bin/main.js ./sheriff
6+
cd ../../
7+
cp sheriff.config.ts sheriff.config.ts.original
38

49
# CLI List Check
510
echo 'checking for CLI list'
6-
sheriff list src/main.ts > tests/actual/cli-list.txt
11+
npx sheriff list src/main.ts > tests/actual/cli-list.txt
712
diff tests/actual/cli-list.txt tests/expected/cli-list.txt
813

914
# CLI Export Check
1015
echo 'checking for CLI export'
11-
sheriff export src/main.ts > tests/actual/cli-export.txt
16+
npx sheriff export src/main.ts > tests/actual/cli-export.txt
1217
diff tests/actual/cli-export.txt tests/expected/cli-export.txt
1318

1419
# CLI Verify Check
1520
echo 'checking for CLI verify'
16-
sheriff verify src/main.ts > tests/actual/cli-verify.txt
21+
npx sheriff verify src/main.ts > tests/actual/cli-verify.txt
1722
diff tests/actual/cli-verify.txt tests/expected/cli-verify.txt
1823

1924
# Dynamic Import Check
@@ -22,36 +27,38 @@ cp tests/dynamic-import-sheriff.config.ts sheriff.config.ts
2227
npx ng lint --force --format json --output-file tests/actual/dynamic-import-lint.json
2328
../remove-paths.mjs tests/actual/dynamic-import-lint.json
2429
diff tests/actual/dynamic-import-lint.json tests/expected/dynamic-import-lint.json
25-
git checkout -q sheriff.config.ts
30+
cp sheriff.config.ts.original sheriff.config.ts
2631

2732
## Deep Import Check
2833
echo 'checking for deep import error'
34+
mv src/app/customers/feature/components/customers-container.component.ts src/app/customers/feature/components/customers-container.component.ts.original
2935
cp tests/customers-container.deep-import.component.ts src/app/customers/feature/components/customers-container.component.ts
3036
npx ng lint --force --format json --output-file tests/actual/deep-import-lint.json
3137
../remove-paths.mjs tests/actual/deep-import-lint.json
3238
diff tests/actual/deep-import-lint.json tests/expected/deep-import-lint.json
33-
git checkout -q src/app/customers/feature/components/customers-container.component.ts
39+
mv src/app/customers/feature/components/customers-container.component.ts.original src/app/customers/feature/components/customers-container.component.ts
3440

3541
## Dependency Rule Check
3642
echo 'checking for dependency rule error'
43+
mv src/app/customers/ui/customer/customer.component.ts src/app/customers/ui/customer/customer.component.ts.original
3744
cp tests/customer.dependency-rule.component.ts src/app/customers/ui/customer/customer.component.ts
3845
npx ng lint --force --format json --output-file tests/actual/dependency-rule-lint.json
3946
../remove-paths.mjs tests/actual/dependency-rule-lint.json
4047
diff tests/actual/dependency-rule-lint.json tests/expected/dependency-rule-lint.json
41-
git checkout -q src/app/customers/ui/customer/customer.component.ts
48+
mv src/app/customers/ui/customer/customer.component.ts.original src/app/customers/ui/customer/customer.component.ts
4249

4350
## User Error Processing
4451
echo 'checking for user error processing'
4552
cp tests/empty-sheriff.config.ts sheriff.config.ts
4653
npx ng lint --force --format json --output-file tests/actual/user-error-lint.json
4754
../remove-paths.mjs tests/actual/user-error-lint.json
4855
diff tests/actual/user-error-lint.json tests/expected/user-error-lint.json
49-
git checkout -q sheriff.config.ts
56+
cp sheriff.config.ts.original sheriff.config.ts
5057

5158
## Auto Tagging
5259
echo 'checking for auto tagging'
5360
cp tests/auto-tagging.config.ts sheriff.config.ts
5461
npx ng lint --force --format json --output-file tests/actual/auto-tagging-lint.json
5562
../remove-paths.mjs tests/actual/auto-tagging-lint.json
5663
diff tests/actual/auto-tagging-lint.json tests/expected/auto-tagging-lint.json
57-
git checkout -q sheriff.config.ts
64+
cp sheriff.config.ts.original sheriff.config.ts

test-projects/angular-i/tests/customers-container.deep-import.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { AsyncPipe, NgIf } from '@angular/common';
22
import { Component, inject } from '@angular/core';
33
import { CustomersComponent, CustomersViewModel } from '@eternal/customers/ui';
4-
import { CustomersRepository } from '../../data/customers-repository.service.ts';
4+
import { CustomersRepository } from '../../data/customers-repository.service';
55
import { Observable } from 'rxjs';
66
import { map } from 'rxjs/operators';
77

test-projects/angular-i/tests/expected/deep-import-lint.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"column": 1,
1111
"nodeType": "ImportDeclaration",
1212
"endLine": 4,
13-
"endColumn": 82
13+
"endColumn": 79
1414
}
1515
],
1616
"suppressedMessages": [],
@@ -19,7 +19,7 @@
1919
"warningCount": 0,
2020
"fixableErrorCount": 0,
2121
"fixableWarningCount": 0,
22-
"source": "import { AsyncPipe, NgIf } from '@angular/common';\nimport { Component, inject } from '@angular/core';\nimport { CustomersComponent, CustomersViewModel } from '@eternal/customers/ui';\nimport { CustomersRepository } from '../../data/customers-repository.service.ts';\nimport { Observable } from 'rxjs';\nimport { map } from 'rxjs/operators';\n\n@Component({\n template: ` <eternal-customers\n *ngIf=\"viewModel$ | async as viewModel\"\n [viewModel]=\"viewModel\"\n (setSelected)=\"setSelected($event)\"\n (setUnselected)=\"setUnselected()\"\n (switchPage)=\"switchPage($event)\"\n ></eternal-customers>`,\n standalone: true,\n imports: [CustomersComponent, NgIf, AsyncPipe],\n})\nexport class CustomersContainerComponent {\n #customersRepository = inject(CustomersRepository);\n viewModel$: Observable<CustomersViewModel> =\n this.#customersRepository.pagedCustomers$.pipe(\n map((pagedCustomers) => ({\n customers: pagedCustomers.customers,\n pageIndex: pagedCustomers.page - 1,\n length: pagedCustomers.total,\n }))\n );\n\n setSelected(id: number) {\n this.#customersRepository.select(id);\n }\n\n setUnselected() {\n this.#customersRepository.unselect();\n }\n\n switchPage(page: number) {\n console.log('switch to page ' + page + ' is not implemented');\n }\n}\n",
22+
"source": "import { AsyncPipe, NgIf } from '@angular/common';\nimport { Component, inject } from '@angular/core';\nimport { CustomersComponent, CustomersViewModel } from '@eternal/customers/ui';\nimport { CustomersRepository } from '../../data/customers-repository.service';\nimport { Observable } from 'rxjs';\nimport { map } from 'rxjs/operators';\n\n@Component({\n template: ` <eternal-customers\n *ngIf=\"viewModel$ | async as viewModel\"\n [viewModel]=\"viewModel\"\n (setSelected)=\"setSelected($event)\"\n (setUnselected)=\"setUnselected()\"\n (switchPage)=\"switchPage($event)\"\n ></eternal-customers>`,\n standalone: true,\n imports: [CustomersComponent, NgIf, AsyncPipe],\n})\nexport class CustomersContainerComponent {\n #customersRepository = inject(CustomersRepository);\n viewModel$: Observable<CustomersViewModel> =\n this.#customersRepository.pagedCustomers$.pipe(\n map((pagedCustomers) => ({\n customers: pagedCustomers.customers,\n pageIndex: pagedCustomers.page - 1,\n length: pagedCustomers.total,\n }))\n );\n\n setSelected(id: number) {\n this.#customersRepository.select(id);\n }\n\n setUnselected() {\n this.#customersRepository.unselect();\n }\n\n switchPage(page: number) {\n console.log('switch to page ' + page + ' is not implemented');\n }\n}\n",
2323
"usedDeprecatedRules": []
2424
}
2525
]

0 commit comments

Comments
 (0)