Skip to content

Commit 92d506e

Browse files
committed
Add unit tests for CDS extractor TypeScript code
Adds unit tests, and unit testing configs, for CDS extractor `extractor/cds/tools/src/*.ts` files. Sets up test coverage reporting as part of an updated `npm run build:all` command for the CDS extractor project. Updates other project configs to supporte the "Testability" phase of a multi-stage rewrite process.
1 parent 152f2a3 commit 92d506e

19 files changed

+8870
-4389
lines changed

extractors/cds/tools/.eslintrc.js

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ module.exports = {
1919
es2018: true
2020
},
2121
ignorePatterns: [
22-
'index-files.js*',
22+
'out/**',
2323
'node_modules',
24-
'*.js.map',
24+
'coverage',
2525
'*.d.ts'
2626
],
2727
rules: {
@@ -73,7 +73,9 @@ module.exports = {
7373
parserOptions: {
7474
ecmaVersion: 2018,
7575
sourceType: 'module',
76-
project: './tsconfig.json'
76+
project: './tsconfig.json',
77+
tsconfigRootDir: __dirname,
78+
createDefaultProgram: true // This helps with files not directly included in the tsconfig
7779
},
7880
settings: {
7981
'import/resolver': {
@@ -85,5 +87,27 @@ module.exports = {
8587
'extensions': ['.js', '.jsx', '.ts', '.tsx']
8688
}
8789
}
88-
}
90+
},
91+
overrides: [
92+
{
93+
files: ['*.ts'],
94+
parserOptions: {
95+
project: './tsconfig.json',
96+
tsconfigRootDir: __dirname
97+
}
98+
},
99+
{
100+
files: ['*.test.ts', 'test/**/*.ts', 'index-files.ts'],
101+
parserOptions: {
102+
project: './tsconfig.json',
103+
tsconfigRootDir: __dirname
104+
},
105+
rules: {
106+
'@typescript-eslint/explicit-function-return-type': 'off',
107+
'@typescript-eslint/no-unsafe-assignment': 'off',
108+
'@typescript-eslint/no-unsafe-call': 'off',
109+
'@typescript-eslint/no-unsafe-member-access': 'off'
110+
}
111+
}
112+
]
89113
}

extractors/cds/tools/.gitignore

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Ignore the entire "out" directory as this is for the .js and .js.map files
2+
# which are generated by the `tsc` build process. In the current project config,
3+
# we require the platform-specific "index-files" shell/cmd script to run the
4+
# `npm run build` command that generates the files for the correct platform and
5+
# local environment.
6+
out/
7+

extractors/cds/tools/autobuild.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@ This document is meant to be a common reference and a project guide while the it
1515
The current extractor for [CDS] is based on `index-files`, which has several limitations and challenges:
1616

1717
1. **Testability**: The current extractor is difficult to test, and especially difficult to troubleshoot when tests fail, because the current implementation lacks unit tests and relieas heavily on integration tests that are performed in a post-commit workflow that runs via GitHub Actions, which makes it more difficult to track errors back to the source of the problem and adds significant delay to the development process.
18+
1819
2. **Performance**: The current extractor is slow and inefficient, especially when dealing with large projects or complex [CDS] files. This is due to the way `index-files` processes files, which can lead to long processing times and increased resource usage. There are several performance improvements that could be made to the extractor, but they are all related to avoid work that we either do not need to do or that has already been done.
1920

2021
- As one example of a performance problem, using the `index-files` approach means that we are provided with a list of all `.cds` files in the project and are expected to index them all, which makes sense for CodeQL (as we want our database to have a copy of every in-scope source code file) but is horribly inefficient from a [CDS] perspective as the [CDS] format allows for a single file to contain multiple [CDS] definitions. The extractor is expected to be able to handle this by parsing the declarative syntax of the `.cds` file in order to understand which other `.cds` files are to be imported as part of that top-level file, meaning that we are expected to avoid duplicate imports of files that are already (and only) used as library-style imports in top-level (project-level) [CDS] files. This is a non-trivial task, and the current extractor does not even try to parse the contents of the `.cds` files to determine which files are actually used in the project. Instead, it simply imports all `.cds` files that are found in the project, which can lead to duplicate imports and increased processing times.
2122

2223
- Another example of a performance problem is that the current `index-files`-based extractor spends a lot of time installing node dependencies because it runs a `npm install` command in every "CDS project directory" that it finds, which is every directory that contains a `package.json` file and either directly contains a `.cds` file (as a sibling of the `package.json` file) or contains some subdirectory that contains either a `.cds` file or a subdirectory that contains a `.cds` file. This means that the extractor will install these dependencies in a directory that we would rather not make changes in just to be able to use a specific version of `@sap/cds` and/or `@sap/cds-dk` (the dependencies that are needed to run the extractor). This also means that if we have five project that all use the same version of `@sap/cds` and/or `@sap/cds-dk`, we will install that version five separate times in five separate locations, which is both a waste of time and creates a cleanup challenge as the install makes changes to the `package-lock.json` file in each of those five project directories (and also makes changes to the `node_modules` subdirectory of each project directory).
2324

2425
3. **Modularity**: The current extractor is mostly just one giant script, aka [index-files.js](./index-files.js), which is surrounded by a collection of small wrapper scripts (aka [index-files.sh](./index-files.sh) and [index-files.cmd](./index-files.cmd)) that are used to allow the JavaScript code to be run in different environments (i.e. Windows and Unix-like environments). While we cannot really get away from the wrapper scripts. we should refactor the "one giant script" (in a single `index-files.js` file) into a more modular design that allows us to break the extractor into smaller, more manageable pieces.
26+
2527
4. **Maintainability**: The current implementation is lacking in terms of mandating consistent code style and best practices. For example, there are no linting rules applied or any scripts for applying consistent code style. This makes it difficult to maintain the code at a consistent level of quality, where it would be much better to have basic linting applied as a pre-commit task (i.e. to be performed in the developer's IDE). The current implementation also lacks documentation, which makes it difficult for new developers to understand how the extractor works and how to contribute to it.
2628

2729
## Goals for the Future Extractor (using `autobuild`)

extractors/cds/tools/index-files.cmd

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ if %ERRORLEVEL% neq 0 (
2020
set "_response_file_path=%~1"
2121
set "_script_dir=%~dp0"
2222
REM Set _cwd before changing the working directory to the script directory.
23+
REM We assume this script is called from the source root directory of the
24+
REM to be scanned project.
2325
set "_cwd=%CD%"
2426

2527
echo Checking response file for CDS files to index
@@ -48,7 +50,9 @@ REM the original working (aka the project source root) directory.
4850
cd /d "%_script_dir%" && ^
4951
echo Installing node package dependencies && ^
5052
npm install --quiet --no-audit --no-fund && ^
53+
echo Building TypeScript code && ^
54+
npm run build && ^
5155
echo Running the 'index-files.js' script && ^
52-
node "%_script_dir%index-files.js" "%_response_file_path%" "%_cwd%"
56+
node "%_script_dir%out\index-files.js" "%_response_file_path%" "%_cwd%"
5357

5458
exit /b %ERRORLEVEL%

extractors/cds/tools/index-files.js

Lines changed: 0 additions & 98 deletions
This file was deleted.

extractors/cds/tools/index-files.js.map

Lines changed: 0 additions & 1 deletion
This file was deleted.

extractors/cds/tools/index-files.sh

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ then
2020
exit 3
2121
fi
2222

23+
# Set the _cwd variable to the present working directory (PWD) as the directory
24+
# from which this script was called, which we assume is the "sourc rooot" directory
25+
# of the project that to be scanned / indexed.
2326
_cwd="$PWD"
2427
_response_file_path="$1"
2528
_script_dir=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
@@ -49,10 +52,12 @@ fi
4952
# 3. passing the original working directory as a parameter to the
5053
# index-files.js script;
5154
# 4. expecting the index-files.js script to immediately change back to
52-
# the original working (aka the project source root) directory.
55+
# original working (aka the project source root) directory.
5356

5457
cd "$_script_dir" && \
5558
echo "Installing node package dependencies" && \
5659
npm install --quiet --no-audit --no-fund && \
60+
echo "Building TypeScript code" && \
61+
npm run build && \
5762
echo "Running the 'index-files.js' script" && \
58-
node "$(dirname "$0")/index-files.js" "$_response_file_path" "${_cwd}"
63+
node "$(dirname "$0")/out/index-files.js" "$_response_file_path" "${_cwd}"

extractors/cds/tools/jest.config.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/** @type {import('ts-jest').JestConfigWithTsJest} */
2+
module.exports = {
3+
preset: 'ts-jest',
4+
testEnvironment: 'node',
5+
testMatch: ['**/test/**/*.test.ts'],
6+
roots: ['<rootDir>/src/', '<rootDir>/test/'],
7+
collectCoverageFrom: [
8+
'src/**/*.ts',
9+
'!src/**/*.d.ts',
10+
'!**/node_modules/**',
11+
],
12+
coverageReporters: ['text', 'lcov', 'clover', 'json'],
13+
coverageDirectory: 'coverage',
14+
verbose: true,
15+
moduleDirectories: ['node_modules', 'src'],
16+
moduleFileExtensions: ['ts', 'js', 'json', 'node'],
17+
transform: {
18+
'^.+\\.ts$': [
19+
'ts-jest',
20+
{
21+
tsconfig: 'tsconfig.json',
22+
diagnostics: {
23+
warnOnly: true
24+
},
25+
isolatedModules: true,
26+
}
27+
]
28+
}
29+
};

0 commit comments

Comments
 (0)