From c4ffc909fddec1fed61ad4b6f5a1e156c36bdfbe Mon Sep 17 00:00:00 2001 From: Mateusz Duda Date: Wed, 22 May 2024 12:48:56 +0200 Subject: [PATCH] added unit tests for report generator --- README.md | 17 +-- src/FileSystem/fileSystemUtils.ts | 12 +- src/Git/GitRepository.ts | 4 + src/References/TSReferenceFinder.ts | 7 +- src/Report/ReportGenerator.ts | 4 +- test/_repo | 2 +- test/report/reportGenerator.test.ts | 219 ++++++++++++++++++++++++++-- 7 files changed, 235 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 19f5ecb..8fea54f 100644 --- a/README.md +++ b/README.md @@ -117,16 +117,13 @@ Publishing is made automatically by pushing a commit to the main branch, see [gi The mock repo can be updated automatically by running `./pushTestRepo.sh` -### Features to do +### Features to do in order of importance -- [ ] Add keyboard shortcut hints when selecting tags and files -> https://github.com/enquirer/enquirer#select-choices -- [ ] Add groups on select prompt: - - [ ] Group files based on common path (files from same directory sould be grouped) - - [ ] Group tags based on parent modules -- [ ] Add remove hanging tags option to tag manager - search for tags not assigned to any module and ask the user if they want to delete them -- [ ] Unit tests for common actions: +- [ ] Find a way to clone test repository locally - this will make unit testing much quicker +- [ ] Unit tests for: + - [ ] The basic actions which can be performed on files - adding, deleting, modifying, renaming. After initial database entry the script should automatically handle all cases. - [ ] Testing if files are correstly updated in database depending on changes in git - [ ] On loading `tags.json` assert that all parents exist in database, if not then these modules won't be displayed -- [ ] Add unit tests for even the basic stuff - reading and parsing JSON files, synchronization between the database and repository, etc. -- [ ] Add unit tests for the basic actions which can be performed on files - adding, deleting, modifying, renaming. After initial database entry the script should automatically handle all cases. -- [ ] Add [adf-validator](https://github.com/torifat/adf-validator/tree/master) which would give more specific errors (right now comments are just not being posted) \ No newline at end of file + - [ ] Reading and parsing JSON files, synchronization between the database and repository, etc. +- [ ] Add remove hanging tags option to tag manager - search for tags not assigned to any module and ask the user if they want to delete them +- [ ] Add keyboard shortcut hints when selecting tags and files -> https://github.com/enquirer/enquirer#select-choices \ No newline at end of file diff --git a/src/FileSystem/fileSystemUtils.ts b/src/FileSystem/fileSystemUtils.ts index 2453553..1c8cfe1 100644 --- a/src/FileSystem/fileSystemUtils.ts +++ b/src/FileSystem/fileSystemUtils.ts @@ -1,5 +1,5 @@ import fs from "fs"; -import path from "path"; +import path, { join } from "path"; export function scopeFolderExists(root: string): boolean { const scopeFolderPath = path.join(root, ".scope"); @@ -27,8 +27,14 @@ export function getFileDirectoryPath(filePath: string): string { return filePath.substring(0, filePath.lastIndexOf("/")); } -export function fileExists(filePath: string): boolean { - return fs.existsSync(filePath); +export function fileExists(filePath: string, relativeTo?: string): boolean { + let path = filePath; + + if (relativeTo) { + path = join(relativeTo, filePath); + } + + return fs.existsSync(path); } export function isDirectory(filePath: string): boolean { diff --git a/src/Git/GitRepository.ts b/src/Git/GitRepository.ts index 1083976..a8f2d9c 100644 --- a/src/Git/GitRepository.ts +++ b/src/Git/GitRepository.ts @@ -440,4 +440,8 @@ export class GitRepository { return false; } } + + public get root() { + return this._root; + } } diff --git a/src/References/TSReferenceFinder.ts b/src/References/TSReferenceFinder.ts index 29edae6..62b9edf 100644 --- a/src/References/TSReferenceFinder.ts +++ b/src/References/TSReferenceFinder.ts @@ -30,10 +30,13 @@ export class TSReferenceFinder implements IReferenceFinder { const referenceList: Array = []; const languageService = this._project.getLanguageService(); - const sourceFile = this._project.getSourceFile(fileNameOrPath); + const allSourceFiles = this._project.getSourceFiles(); + + const pathRelativeToRoot = path.join(this._root, fileNameOrPath); + const sourceFile = this._project.getSourceFile(pathRelativeToRoot); if (!sourceFile) { - console.log(`[TSReferenceFinder] File '${fileNameOrPath}' is not in scope of tsconfig.json of the project.`); + console.log(`[TSReferenceFinder] File '${pathRelativeToRoot}' is not in scope of tsconfig.json of the project.`); return []; } diff --git a/src/Report/ReportGenerator.ts b/src/Report/ReportGenerator.ts index 1f54158..ed5ddc9 100644 --- a/src/Report/ReportGenerator.ts +++ b/src/Report/ReportGenerator.ts @@ -188,7 +188,8 @@ export class ReportGenerator { private _getFileReferences(file: string, relevancy: Relevancy | null): Array { const references: Array = []; - if (!fileExists(file)) { + // Command should be run outside of repo, so check path relative to repo + if (!fileExists(file, this._repository.root)) { return references; } @@ -196,6 +197,7 @@ export class ReportGenerator { if (!referenceFinder.getSupportedFilesExtension().includes(getExtension(file))) { return; } + const foundReferences = referenceFinder.findReferences(file, relevancy); foundReferences.forEach(reference => { diff --git a/test/_repo b/test/_repo index 1a6f4d7..903bdea 160000 --- a/test/_repo +++ b/test/_repo @@ -1 +1 @@ -Subproject commit 1a6f4d7d908158c595c4b08755bec54577ab1518 +Subproject commit 903bdea9ba64e3425cb8c277a175a8e8d183e219 diff --git a/test/report/reportGenerator.test.ts b/test/report/reportGenerator.test.ts index f2f2b58..6129a13 100644 --- a/test/report/reportGenerator.test.ts +++ b/test/report/reportGenerator.test.ts @@ -6,6 +6,7 @@ import { TagsDefinitionFile } from "../../src/Scope/TagsDefinitionFile"; import { TSReferenceFinder } from "../../src/References/TSReferenceFinder"; import { ReportGenerator } from "../../src/Report/ReportGenerator"; import { RelevancyManager } from "../../src/Relevancy/RelevancyManager"; +import { Relevancy } from "../../src/Relevancy/Relevancy"; // Testing only ReportGenerator class, which is responsible for gathering data for each commit @@ -50,9 +51,7 @@ describe("Report generation works as expected", async () => { expect(report).toBeDefined(); - // { "path": "src/tagged-file.js", - // "tags": [ - // { "tag": "Tag", "module": "Default module" }]}], + // src/tagged-file.js has 1 tag -> Default module / Tag expect(report.allModules.length).toBe(1); expect(report.untaggedFilesAsModule.files.length).toBe(0); @@ -136,7 +135,6 @@ describe("Report generation works as expected", async () => { }); }); - it("Correctly separates tagged and untagged files in the report", async () => { const FOLDER_PATH = makeUniqueFolderForTest(); const REPO_PATH = cloneMockRepositoryToFolder(FOLDER_PATH); @@ -177,13 +175,12 @@ describe("Report generation works as expected", async () => { * src/ * ├─ ts/ * │ ├─ controllers/ - * │ │ ├─ Controller.ts 2 dependencies: View.ts and Model.ts, 1 tag: Controllers / AController + * │ │ ├─ Controller.ts 3 dependencies: View.ts and Model.ts, 1 tag: Controllers / AController * │ ├─ models/ - * │ │ ├─ Model.ts No dependencies, 1 tag: Models / AModel + * │ │ ├─ Model.ts 1 dependency: View.ts, 1 tag: Models / AModel * │ ├─ views/ - * │ │ ├─ View.ts 1 dependency: Modal, 1 tag: Views / AView - * │ │ ├─ ModalWindow.ts No dependencies, no tags - * + * │ │ ├─ View.ts 1 dependency: ModalWindow.ts, 1 tag: Views / AView + * │ │ ├─ ModalWindow.ts No dependencies, no tags */ const filesToModify = [ @@ -251,14 +248,210 @@ describe("Report generation works as expected", async () => { // View.ts checks const viewFileInfo = viewsModule.files[0]; - expect(viewFileInfo.file).toBe("src/ts/view/View.ts"); + expect(viewFileInfo.file).toBe("src/ts/views/View.ts"); + expect(viewFileInfo.ignored).toBe(false); + expect(viewFileInfo.tagIdentifiers.length).toBeGreaterThan(0); + expect(viewFileInfo.tagIdentifiers.some(identifier => identifier.module === "Views" && identifier.tag === "AView")).toBe(true); + + expect(viewFileInfo.usedIn.length).toBe(2); + + const controllerDependency = viewFileInfo.usedIn.find(file => file.fileInfo.filename === "src/ts/controllers/Controller.ts"); + const modelDependency = viewFileInfo.usedIn.find(file => file.fileInfo.filename === "src/ts/models/Model.ts"); + + expect(controllerDependency).toBeDefined(); + expect(modelDependency).toBeDefined(); + + if (!controllerDependency || !modelDependency) { + return; + } + + expect(controllerDependency.fileInfo.filename).toBe("src/ts/controllers/Controller.ts"); + expect(controllerDependency.fileInfo.unused).toBe(false); + + expect(modelDependency.fileInfo.filename).toBe("src/ts/models/Model.ts"); + expect(modelDependency.fileInfo.unused).toBe(false); // Check if ModalWindow is marked as untagged expect(report.untaggedFilesAsModule.files.length).toBe(1); - expect(report.untaggedFilesAsModule.files[0].file).toBe("src/ts/view/ModalWindow.ts"); + expect(report.untaggedFilesAsModule.files[0].file).toBe("src/ts/views/ModalWindow.ts"); + expect(report.untaggedFilesAsModule.files[0].ignored).toBe(false); + expect(report.untaggedFilesAsModule.files[0].tagIdentifiers.length).toBe(0); + + expect(report.untaggedFilesAsModule.files[0].usedIn.length).toBe(1); + + const viewDependency = report.untaggedFilesAsModule.files[0].usedIn[0]; - // Check references + expect(viewDependency.fileInfo.filename).toBe("src/ts/views/View.ts"); + expect(viewDependency.fileInfo.unused).toBe(false); + expect(viewDependency.tagIdentifiers.some(identifier => identifier.module === "Views" && identifier.tag === "AView")).toBe(true); }); -}); + it("Circular dependencies are reported correctly", async () => { + const FOLDER_PATH = makeUniqueFolderForTest(); + const REPO_PATH = cloneMockRepositoryToFolder(FOLDER_PATH); + + /** + * File structure description + * + * src/ + * ├─ ts/ + * │ ├─ circularDependency/ + * │ │ ├─ ModuleA.ts 1 dependency: ModuleB.ts, 1 tag: Circular Dependency Test (Module A) / Module A + * │ │ ├─ ModuleB.ts 1 dependency: ModuleA.ts, 1 tag: Circular Dependency Test (Module B) / Module B + */ + + const filesToModify = [ + "src/ts/circularDependency/ModuleA.ts", + "src/ts/circularDependency/ModuleB.ts", + ]; + + const repository = await commitModitication(filesToModify, REPO_PATH); + + const unpushedCommits = await repository.getUnpushedCommits(); + expect(unpushedCommits.length).toBe(1); + + const commit = unpushedCommits[0]; + + const generator = initReportGenerator(REPO_PATH); + + const relevancy = new RelevancyManager(); + const relevancyMap = relevancy.loadRelevancyMapFromCommits([commit]); + + const report = await generator.generateReportForCommit(commit, "Project", relevancyMap, true); + + expect(report).toBeDefined(); + + expect(report.allModules.length).toBe(2); + + const circularTestModuleA = report.allModules.find(reportModule => reportModule.module === "Circular Dependency Test (Module A)"); + const circularTestModuleB = report.allModules.find(reportModule => reportModule.module === "Circular Dependency Test (Module B)"); + + expect(circularTestModuleA).toBeDefined(); + expect(circularTestModuleB).toBeDefined(); + + if (!circularTestModuleA || !circularTestModuleB) { + return; + } + + expect(circularTestModuleA.files.length).toBe(1); + expect(circularTestModuleA.files[0].file).toBe("src/ts/circularDependency/ModuleA.ts"); + expect(circularTestModuleA.files[0].ignored).toBe(false); + expect(circularTestModuleA.files[0].tagIdentifiers.length).toBe(1); + expect(circularTestModuleA.files[0].tagIdentifiers.some(identifier => identifier.tag === "Module A" && identifier.module === "Circular Dependency Test (Module A)")).toBe(true); + expect(circularTestModuleA.files[0].usedIn.length).toBe(1); + expect(circularTestModuleA.files[0].usedIn[0].fileInfo.filename).toBe("src/ts/circularDependency/ModuleB.ts"); + expect(circularTestModuleA.files[0].usedIn[0].fileInfo.unused).toBe(false); + + expect(circularTestModuleB.files.length).toBe(1); + expect(circularTestModuleB.files[0].file).toBe("src/ts/circularDependency/ModuleB.ts"); + expect(circularTestModuleB.files[0].ignored).toBe(false); + expect(circularTestModuleB.files[0].tagIdentifiers.length).toBe(1); + expect(circularTestModuleB.files[0].tagIdentifiers.some(identifier => identifier.tag === "Module B" && identifier.module === "Circular Dependency Test (Module B)")).toBe(true); + expect(circularTestModuleB.files[0].usedIn.length).toBe(1); + expect(circularTestModuleB.files[0].usedIn[0].fileInfo.filename).toBe("src/ts/circularDependency/ModuleA.ts"); + expect(circularTestModuleB.files[0].usedIn[0].fileInfo.unused).toBe(false); + + // Same for module B + }); + + it("Correctly reports files with multiple tags", async () => { + + const FOLDER_PATH = makeUniqueFolderForTest(); + const REPO_PATH = cloneMockRepositoryToFolder(FOLDER_PATH); + + /** + Has following tags: + - Default module / Second + - Default module / Tag + - Default module 2 / Tag of default module 2 + */ + + const fileToModify = "src/tagged-file-with-multiple-modules.js"; + + const repository = await commitModitication([fileToModify], REPO_PATH); + + const unpushedCommits = await repository.getUnpushedCommits(); + expect(unpushedCommits.length).toBe(1); + + const commit = unpushedCommits[0]; + + const generator = initReportGenerator(REPO_PATH); + + const relevancy = new RelevancyManager(); + const relevancyMap = relevancy.loadRelevancyMapFromCommits([commit]); + + const report = await generator.generateReportForCommit(commit, "Project", relevancyMap, true); + + expect(report).toBeDefined(); + expect(report.allModules.length).toBe(2); + expect(report.untaggedFilesAsModule.files.length).toBe(0); + + const defaultModule = report.allModules.find(reportModule => reportModule.module === "Default module"); + const defaultModule2 = report.allModules.find(reportModule => reportModule.module === "Default module 2"); + + expect(defaultModule).toBeDefined(); + expect(defaultModule2).toBeDefined(); + + if (!defaultModule || !defaultModule2) { + return; + } + + expect(defaultModule.files.length).toBe(1); + expect(defaultModule.files[0].file).toBe("src/tagged-file-with-multiple-modules.js"); + expect(defaultModule.files[0].ignored).toBe(false); + expect(defaultModule.files[0].usedIn.length).toBe(0); + // 3, because same FileInfo is shared between modules + expect(defaultModule.files[0].tagIdentifiers.length).toBe(3); + expect(defaultModule.files[0].tagIdentifiers.some(identifier => identifier.tag === "Tag" && identifier.module === "Default module")).toBe(true); + expect(defaultModule.files[0].tagIdentifiers.some(identifier => identifier.tag === "Second" && identifier.module === "Default module")).toBe(true); + + expect(defaultModule2.files.length).toBe(1); + expect(defaultModule2.files[0].file).toBe("src/tagged-file-with-multiple-modules.js"); + expect(defaultModule2.files[0].ignored).toBe(false); + expect(defaultModule2.files[0].usedIn.length).toBe(0); + // 3, because same FileInfo is shared between modules + expect(defaultModule2.files[0].tagIdentifiers.length).toBe(3); + expect(defaultModule2.files[0].tagIdentifiers.some(identifier => identifier.tag === "Tag of default module 2" && identifier.module === "Default module 2")).toBe(true); + }); + + it("After making a change with defined relevancy, the relevancy is present in generated report", async () => { + const FOLDER_PATH = makeUniqueFolderForTest(); + const REPO_PATH = cloneMockRepositoryToFolder(FOLDER_PATH); + + const repository = await commitModitication( + [ + "src/tagged-file.js", + "src/untagged-file.js" + ], + REPO_PATH, + `Automatic commit + + __relevancy__[{"path":"src/tagged-file.js","relevancy":"HIGH","commit":"__current__"},{"path":"src/untagged-file.js","relevancy":"LOW","commit":"__current__"}]__relevancy__ + `); + + const unpushedCommits = await repository.getUnpushedCommits(); + expect(unpushedCommits.length).toBe(1); + + const commit = unpushedCommits[0]; + + const generator = initReportGenerator(REPO_PATH); + + const relevancy = new RelevancyManager(); + const relevancyMap = relevancy.loadRelevancyMapFromCommits([commit]); + + const report = await generator.generateReportForCommit(commit, "Project", relevancyMap, true); + + expect(report).toBeDefined(); + expect(report.allModules.length).toBe(1); + + const taggedModule = report.allModules[0]; + expect(taggedModule.files.length).toBe(1); + expect(taggedModule.files[0].file).toBe("src/tagged-file.js"); + expect(taggedModule.files[0].relevancy).toBe(Relevancy.HIGH); + + expect(report.untaggedFilesAsModule.files.length).toBe(1); + expect(report.untaggedFilesAsModule.files[0].file).toBe("src/untagged-file.js"); + expect(report.untaggedFilesAsModule.files[0].relevancy).toBe(Relevancy.LOW); + }); +});