From 559a24affc32c22d5c0d17446dad09212dccf315 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 9 Mar 2023 14:49:49 +0100 Subject: [PATCH 01/12] [FIX] ui5Framework: Prevent install of libraries within workspace --- lib/graph/helpers/ui5Framework.js | 67 ++++++++++++++++++++++++++-- lib/ui5Framework/AbstractResolver.js | 6 ++- 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index 9fe94d1bb..9670d7036 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -208,6 +208,57 @@ const utils = { ); } }, + /** + * This logic needs to stay in sync with the @sapui5/distribution-metadata package + * + * @param project + */ + async getFrameworkLibraryDependencies(project) { + let dependencies = []; + let optionalDependencies = []; + + if (project.__id.startsWith("@sapui5/")) { + project.getFrameworkDependencies().forEach((dependency) => { + if (dependency.optional) { + // Add optional dependency to optionalDependencies + optionalDependencies.push(dependency.name); + } else if (!dependency.development) { + // Add non-development dependency to dependencies + dependencies.push(dependency.name); + } + }); + } else if (project.__id.startsWith("@openui5/")) { + const packageResource = await project.getRootReader().byPath("/package.json"); + const packageInfo = JSON.parse(await packageResource.getString()); + + dependencies = Object.keys( + packageInfo.dependencies || {} + ).map(($) => $.replace("@openui5/", "")); // @sapui5 dependencies must not be defined in package.json + optionalDependencies = Object.keys( + packageInfo.devDependencies || {} + ).map(($) => $.replace("@openui5/", "")); // @sapui5 dependencies must not be defined in package.json + } + + return {dependencies, optionalDependencies}; + }, + + async getWorkspaceFrameworkLibraryMetadata({workspace, projectGraph}) { + const libraryMetadata = Object.create(null); + const {projectNameMap} = await workspace._getResolvedModules(); // TODO: private API usage + for (const ui5Module of projectNameMap.values()) { + const {project} = await ui5Module.getSpecifications(); + if (project.isFrameworkProject?.() && !projectGraph.getProject(project.getName())) { + const metadata = libraryMetadata[project.getName()] = Object.create(null); + metadata.id = project.__id; + metadata.path = project.getRootPath(); + metadata.version = project.getVersion(); + const {dependencies, optionalDependencies} = await utils.getFrameworkLibraryDependencies(project); + metadata.dependencies = dependencies; + metadata.optionalDependencies = optionalDependencies; + } + } + return libraryMetadata; + }, ProjectProcessor }; @@ -232,6 +283,7 @@ export default { * Promise resolving with the given graph instance to allow method chaining */ enrichProjectGraph: async function(projectGraph, options = {}) { + const {workspace} = options; const rootProject = projectGraph.getRoot(); const frameworkName = rootProject.getFrameworkName(); const frameworkVersion = rootProject.getFrameworkVersion(); @@ -242,6 +294,8 @@ export default { let version = options.versionOverride || frameworkVersion; if (rootProject.isFrameworkProject() && !version) { + // TODO: Also take workspace into account + // If the root project is a framework project and no framework version is defined, // all framework dependencies must already be part of the graph rootProject.getFrameworkDependencies().forEach((dep) => { @@ -298,14 +352,21 @@ export default { log.info(`Using ${frameworkName} version: ${version}`); - const resolver = new Resolver({cwd: rootProject.getRootPath(), version}); + const resolver = new Resolver({cwd: rootProject.getRootPath(), version, workspace}); let startTime; if (log.isLevelEnabled("verbose")) { startTime = process.hrtime(); } - const {libraryMetadata} = await resolver.install(referencedLibraries); + let workspaceFrameworkLibraryMetadata; + if (workspace) { + workspaceFrameworkLibraryMetadata = await utils.getWorkspaceFrameworkLibraryMetadata({ + workspace, projectGraph + }); + } + + const {libraryMetadata} = await resolver.install(referencedLibraries, workspaceFrameworkLibraryMetadata); if (log.isLevelEnabled("verbose")) { const timeDiff = process.hrtime(startTime); @@ -322,7 +383,7 @@ export default { const projectProcessor = new utils.ProjectProcessor({ libraryMetadata, graph: frameworkGraph, - workspace: options.workspace + workspace }); await Promise.all(referencedLibraries.map(async (libName) => { diff --git a/lib/ui5Framework/AbstractResolver.js b/lib/ui5Framework/AbstractResolver.js index e6e9537be..573c6e093 100644 --- a/lib/ui5Framework/AbstractResolver.js +++ b/lib/ui5Framework/AbstractResolver.js @@ -160,11 +160,15 @@ class AbstractResolver { * * @public * @param {string[]} libraryNames List of library names to be installed + * @param {object} [initialLibraryMetadata] Initial libraryMetadata object to be used * @returns {@ui5/project/ui5Framework/AbstractResolver~ResolverInstallResult} * Resolves with an object containing the libraryMetadata */ - async install(libraryNames) { + async install(libraryNames, initialLibraryMetadata) { const libraryMetadata = Object.create(null); + if (initialLibraryMetadata) { + Object.assign(libraryMetadata, initialLibraryMetadata); + } const errors = []; await this._processLibraries(libraryNames, libraryMetadata, errors); From dd42383bb08d2a5c09591be1b3e444ff94b19698 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 10 Mar 2023 15:27:30 +0100 Subject: [PATCH 02/12] [INTERNAL] Enhance TODO / jsdoc comments --- lib/graph/helpers/ui5Framework.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index 9670d7036..26cf97110 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -209,14 +209,16 @@ const utils = { } }, /** - * This logic needs to stay in sync with the @sapui5/distribution-metadata package + * This logic needs to stay in sync with the dependency definitions for the + * sapui5/distribution-metadata package. * - * @param project + * @param {@ui5/project/specifications/Project} project */ async getFrameworkLibraryDependencies(project) { let dependencies = []; let optionalDependencies = []; + // TODO: Add getter for ID if (project.__id.startsWith("@sapui5/")) { project.getFrameworkDependencies().forEach((dependency) => { if (dependency.optional) { @@ -241,7 +243,6 @@ const utils = { return {dependencies, optionalDependencies}; }, - async getWorkspaceFrameworkLibraryMetadata({workspace, projectGraph}) { const libraryMetadata = Object.create(null); const {projectNameMap} = await workspace._getResolvedModules(); // TODO: private API usage @@ -294,7 +295,8 @@ export default { let version = options.versionOverride || frameworkVersion; if (rootProject.isFrameworkProject() && !version) { - // TODO: Also take workspace into account + // TODO: Also take workspace into account: + // Version should not be mandatory if all framework dependencies are part of the workspace // If the root project is a framework project and no framework version is defined, // all framework dependencies must already be part of the graph From e5a890b70790aa1bf0f7cfc0572e875f3aab28d0 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 10 Mar 2023 15:36:15 +0100 Subject: [PATCH 03/12] [INTERNAL] Don't pass workspace to Resolver constructor / fix tests --- lib/graph/helpers/ui5Framework.js | 2 +- test/lib/graph/helpers/ui5Framework.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index 26cf97110..d6263867d 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -354,7 +354,7 @@ export default { log.info(`Using ${frameworkName} version: ${version}`); - const resolver = new Resolver({cwd: rootProject.getRootPath(), version, workspace}); + const resolver = new Resolver({cwd: rootProject.getRootPath(), version}); let startTime; if (log.isLevelEnabled("verbose")) { diff --git a/test/lib/graph/helpers/ui5Framework.js b/test/lib/graph/helpers/ui5Framework.js index 52426261d..06fc6f4d4 100644 --- a/test/lib/graph/helpers/ui5Framework.js +++ b/test/lib/graph/helpers/ui5Framework.js @@ -105,7 +105,8 @@ test.serial("enrichProjectGraph", async (t) => { t.is(t.context.Sapui5ResolverInstallStub.callCount, 1, "Sapui5Resolver#install should be called once"); t.deepEqual(t.context.Sapui5ResolverInstallStub.getCall(0).args, [ - referencedLibraries + referencedLibraries, + undefined ], "Sapui5Resolver#install should be called with expected args"); t.is(ProjectProcessorStub.callCount, 1, "ProjectProcessor#constructor should be called once"); From d336bc704c7a5ef0677ec5b06192d1acd2ceb0a0 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Mon, 20 Mar 2023 18:03:42 +0100 Subject: [PATCH 04/12] [INTERNAL] Handle dependencies, add tests --- lib/graph/helpers/ui5Framework.js | 18 ++--- lib/ui5Framework/AbstractResolver.js | 25 +++++-- .../graph/helpers/ui5Framework.integration.js | 72 +++++++++++++++++-- test/lib/graph/helpers/ui5Framework.js | 15 ++-- 4 files changed, 104 insertions(+), 26 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index d6263867d..ec7772c1e 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -354,21 +354,21 @@ export default { log.info(`Using ${frameworkName} version: ${version}`); - const resolver = new Resolver({cwd: rootProject.getRootPath(), version}); + let providedLibraryMetadata; + if (workspace) { + providedLibraryMetadata = await utils.getWorkspaceFrameworkLibraryMetadata({ + workspace, projectGraph + }); + } + + const resolver = new Resolver({cwd: rootProject.getRootPath(), version, providedLibraryMetadata}); let startTime; if (log.isLevelEnabled("verbose")) { startTime = process.hrtime(); } - let workspaceFrameworkLibraryMetadata; - if (workspace) { - workspaceFrameworkLibraryMetadata = await utils.getWorkspaceFrameworkLibraryMetadata({ - workspace, projectGraph - }); - } - - const {libraryMetadata} = await resolver.install(referencedLibraries, workspaceFrameworkLibraryMetadata); + const {libraryMetadata} = await resolver.install(referencedLibraries); if (log.isLevelEnabled("verbose")) { const timeDiff = process.hrtime(startTime); diff --git a/lib/ui5Framework/AbstractResolver.js b/lib/ui5Framework/AbstractResolver.js index 573c6e093..f6f87b8dc 100644 --- a/lib/ui5Framework/AbstractResolver.js +++ b/lib/ui5Framework/AbstractResolver.js @@ -34,7 +34,7 @@ class AbstractResolver { * @param {string} [options.ui5HomeDir="~/.ui5"] UI5 home directory location. This will be used to store packages, * metadata and configuration used by the resolvers. Relative to `process.cwd()` */ - constructor({cwd, version, ui5HomeDir}) { + constructor({cwd, version, ui5HomeDir, providedLibraryMetadata}) { if (new.target === AbstractResolver) { throw new TypeError("Class 'AbstractResolver' is abstract"); } @@ -50,6 +50,7 @@ class AbstractResolver { ); this._cwd = cwd ? path.resolve(cwd) : process.cwd(); this._version = version; + this._providedLibraryMetadata = providedLibraryMetadata; } async _processLibrary(libraryName, libraryMetadata, errors) { @@ -62,7 +63,21 @@ class AbstractResolver { log.verbose("Processing " + libraryName); - const promises = await this.handleLibrary(libraryName); + let promises; + const providedLibraryMetadata = this._providedLibraryMetadata?.[libraryName]; + if (providedLibraryMetadata) { + log.verbose(`Skipping install for ${libraryName} (provided)`); + promises = { + // Use existing metadata if library is provided from outside (e.g. workspace) + metadata: Promise.resolve(providedLibraryMetadata), + // Provided libraries are already "installed" + install: Promise.resolve({ + pkgPath: providedLibraryMetadata.path + }) + }; + } else { + promises = await this.handleLibrary(libraryName); + } const [metadata, {pkgPath}] = await Promise.all([ promises.metadata.then((metadata) => @@ -160,15 +175,11 @@ class AbstractResolver { * * @public * @param {string[]} libraryNames List of library names to be installed - * @param {object} [initialLibraryMetadata] Initial libraryMetadata object to be used * @returns {@ui5/project/ui5Framework/AbstractResolver~ResolverInstallResult} * Resolves with an object containing the libraryMetadata */ - async install(libraryNames, initialLibraryMetadata) { + async install(libraryNames) { const libraryMetadata = Object.create(null); - if (initialLibraryMetadata) { - Object.assign(libraryMetadata, initialLibraryMetadata); - } const errors = []; await this._processLibraries(libraryNames, libraryMetadata, errors); diff --git a/test/lib/graph/helpers/ui5Framework.integration.js b/test/lib/graph/helpers/ui5Framework.integration.js index 132c58461..d354d0cbd 100644 --- a/test/lib/graph/helpers/ui5Framework.integration.js +++ b/test/lib/graph/helpers/ui5Framework.integration.js @@ -135,7 +135,8 @@ test.afterEach.always((t) => { function defineTest(testName, { frameworkName, - verbose = false + verbose = false, + librariesInWorkspace }) { const npmScope = frameworkName === "SAPUI5" ? "@sapui5" : "@openui5"; @@ -410,7 +411,61 @@ function defineTest(testName, { const provider = new DependencyTreeProvider({dependencyTree}); const projectGraph = await projectGraphBuilder(provider); - await ui5Framework.enrichProjectGraph(projectGraph); + if (librariesInWorkspace) { + const projectNameMap = new Map(); + const moduleIdMap = new Map(); + librariesInWorkspace.forEach((libName) => { + const libraryDistMetadata = distributionMetadata.libraries[libName]; + const module = { + getSpecifications: sinon.stub().resolves({ + project: { + getName: sinon.stub().returns(libName), + getVersion: sinon.stub().returns("1.76.0-SNAPSHOT"), + getRootPath: sinon.stub().returns(path.join(fakeBaseDir, "workspace", libName)), + isFrameworkProject: sinon.stub().returns(true), + __id: libraryDistMetadata.npmPackageName, + getRootReader: sinon.stub().returns({ + byPath: sinon.stub().resolves({ + getString: sinon.stub().resolves(JSON.stringify({dependencies: {}})) + }) + }), + getFrameworkDependencies: sinon.stub().callsFake(() => { + const deps = []; + libraryDistMetadata.dependencies.forEach((dep) => { + deps.push({name: dep}); + }); + libraryDistMetadata.optionalDependencies.forEach((optDep) => { + deps.push({name: optDep, optional: true}); + }); + return deps; + }), + isDeprecated: sinon.stub().returns(false), + isSapInternal: sinon.stub().returns(false), + getAllowSapInternal: sinon.stub().returns(false), + } + }), + getVersion: sinon.stub().returns("1.76.0-SNAPSHOT"), + getPath: sinon.stub().returns(path.join(fakeBaseDir, "workspace", libName)), + }; + projectNameMap.set(libName, module); + moduleIdMap.set(libraryDistMetadata.npmPackageName); + }); + + const getModuleByProjectName = sinon.stub().callsFake((projectName) => projectNameMap.get(projectName)); + + const workspace = { + getName: sinon.stub().returns("test"), + _getResolvedModules: sinon.stub().resolves({ + projectNameMap, + moduleIdMap + }), + getModuleByProjectName + }; + + await ui5Framework.enrichProjectGraph(projectGraph, {workspace}); + } else { + await ui5Framework.enrichProjectGraph(projectGraph); + } const callbackStub = sinon.stub().resolves(); await projectGraph.traverseDepthFirst(callbackStub); @@ -465,6 +520,15 @@ defineTest("ui5Framework helper should enhance project graph with UI5 framework verbose: true }); +defineTest("ui5Framework helper should not cause install of libraries within workspace", { + frameworkName: "SAPUI5", + librariesInWorkspace: ["sap.ui.lib1", "sap.ui.lib2", "sap.ui.lib8"] +}); +defineTest("ui5Framework helper should not cause install of libraries within workspace", { + frameworkName: "OpenUI5", + librariesInWorkspace: ["sap.ui.lib1", "sap.ui.lib2", "sap.ui.lib8"] +}); + function defineErrorTest(testName, { frameworkName, failExtract = false, @@ -723,7 +787,7 @@ test.serial("ui5Framework translator should not try to install anything when no t.is(pacote.manifest.callCount, 0, "No manifest should be requested"); }); -test.serial("ui5Framework translator should throw an error when framework version is not defined", async (t) => { +test.serial("ui5Framework helper should throw an error when framework version is not defined", async (t) => { const {ui5Framework, projectGraphBuilder} = t.context; const dependencyTree = { @@ -751,7 +815,7 @@ test.serial("ui5Framework translator should throw an error when framework versio }); test.serial( - "SAPUI5: ui5Framework translator should throw error when using a library that is not part of the dist metadata", + "SAPUI5: ui5Framework helper should throw error when using a library that is not part of the dist metadata", async (t) => { const {sinon, ui5Framework, Installer, projectGraphBuilder} = t.context; diff --git a/test/lib/graph/helpers/ui5Framework.js b/test/lib/graph/helpers/ui5Framework.js index 06fc6f4d4..b1237d153 100644 --- a/test/lib/graph/helpers/ui5Framework.js +++ b/test/lib/graph/helpers/ui5Framework.js @@ -100,13 +100,13 @@ test.serial("enrichProjectGraph", async (t) => { t.is(t.context.Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once"); t.deepEqual(t.context.Sapui5ResolverStub.getCall(0).args, [{ cwd: dependencyTree.path, - version: dependencyTree.configuration.framework.version + version: dependencyTree.configuration.framework.version, + providedLibraryMetadata: undefined }], "Sapui5Resolver#constructor should be called with expected args"); t.is(t.context.Sapui5ResolverInstallStub.callCount, 1, "Sapui5Resolver#install should be called once"); t.deepEqual(t.context.Sapui5ResolverInstallStub.getCall(0).args, [ - referencedLibraries, - undefined + referencedLibraries ], "Sapui5Resolver#install should be called with expected args"); t.is(ProjectProcessorStub.callCount, 1, "ProjectProcessor#constructor should be called once"); @@ -187,7 +187,8 @@ test.serial("enrichProjectGraph: With versionOverride", async (t) => { t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once"); t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{ cwd: dependencyTree.path, - version: "1.99.9" + version: "1.99.9", + providedLibraryMetadata: undefined }], "Sapui5Resolver#constructor should be called with expected args"); }); @@ -315,7 +316,8 @@ test.serial("enrichProjectGraph should resolve framework project with version an t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once"); t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{ cwd: dependencyTree.path, - version: "1.2.3" + version: "1.2.3", + providedLibraryMetadata: undefined }], "Sapui5Resolver#constructor should be called with expected args"); }); @@ -394,7 +396,8 @@ test.serial("enrichProjectGraph should resolve framework project " + t.is(getFrameworkLibrariesFromGraphStub.callCount, 1, "getFrameworkLibrariesFromGrap should be called once"); t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{ cwd: dependencyTree.path, - version: "1.99.9" + version: "1.99.9", + providedLibraryMetadata: undefined }], "Sapui5Resolver#constructor should be called with expected args"); }); From 3cadc8feb93a642b6a71e0da3943965f29a57d7e Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Tue, 28 Mar 2023 15:36:09 +0200 Subject: [PATCH 05/12] [INTERNAL] Don't require framework version when no install is needed --- lib/graph/helpers/ui5Framework.js | 31 ++- lib/ui5Framework/AbstractResolver.js | 6 +- .../graph/helpers/ui5Framework.integration.js | 27 ++- test/lib/graph/helpers/ui5Framework.js | 209 +++++++++++++++++- test/lib/ui5framework/AbstractResolver.js | 7 - 5 files changed, 236 insertions(+), 44 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index ec7772c1e..5b253053a 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -289,25 +289,26 @@ export default { const frameworkName = rootProject.getFrameworkName(); const frameworkVersion = rootProject.getFrameworkVersion(); - // It is allowed omit the framework version in ui5.yaml and only provide one via the override + // It is allowed to omit the framework version in ui5.yaml and only provide one via the override // This is a common use case for framework libraries, which generally should not define a // framework version in their respective ui5.yaml let version = options.versionOverride || frameworkVersion; if (rootProject.isFrameworkProject() && !version) { - // TODO: Also take workspace into account: - // Version should not be mandatory if all framework dependencies are part of the workspace - // If the root project is a framework project and no framework version is defined, - // all framework dependencies must already be part of the graph - rootProject.getFrameworkDependencies().forEach((dep) => { - if (utils.shouldIncludeDependency(dep) && !projectGraph.getProject(dep.name)) { - throw new Error( - `Missing framework dependency ${dep.name} for framework project ${rootProject.getName()}`); - } + // all framework dependencies must either be already part of the graph or part of the workspace. + // A mixed setup of framework deps within the graph AND from the workspace is currently not supported. + + const someDependencyMissing = rootProject.getFrameworkDependencies().some((dep) => { + return utils.shouldIncludeDependency(dep) && !projectGraph.getProject(dep.name); }); - // All framework dependencies are already present in the graph - return projectGraph; + + // If all dependencies are available there is nothing else to do here. + // In case of a workspace setup, the resolver will be created below without a version and + // will succeed in case no library needs to be actually installed. + if (!someDependencyMissing) { + return projectGraph; + } } @@ -323,12 +324,6 @@ export default { ); } - if (!version) { - throw new Error( - `No framework version defined for root project ${rootProject.getName()}` - ); - } - let Resolver; if (frameworkName === "OpenUI5") { Resolver = (await import("../../ui5Framework/Openui5Resolver.js")).default; diff --git a/lib/ui5Framework/AbstractResolver.js b/lib/ui5Framework/AbstractResolver.js index f6f87b8dc..0fbf09dc2 100644 --- a/lib/ui5Framework/AbstractResolver.js +++ b/lib/ui5Framework/AbstractResolver.js @@ -39,10 +39,6 @@ class AbstractResolver { throw new TypeError("Class 'AbstractResolver' is abstract"); } - if (!version) { - throw new Error(`AbstractResolver: Missing parameter "version"`); - } - // In some CI environments, the homedir might be set explicitly to a relative // path (e.g. "./"), but tooling requires an absolute path this._ui5HomeDir = path.resolve( @@ -75,6 +71,8 @@ class AbstractResolver { pkgPath: providedLibraryMetadata.path }) }; + } else if (!this._version) { + throw new Error(`Unable to install library ${libraryName}. No framework version provided.`); } else { promises = await this.handleLibrary(libraryName); } diff --git a/test/lib/graph/helpers/ui5Framework.integration.js b/test/lib/graph/helpers/ui5Framework.integration.js index d354d0cbd..9e2cac146 100644 --- a/test/lib/graph/helpers/ui5Framework.integration.js +++ b/test/lib/graph/helpers/ui5Framework.integration.js @@ -136,7 +136,7 @@ test.afterEach.always((t) => { function defineTest(testName, { frameworkName, verbose = false, - librariesInWorkspace + librariesInWorkspace = null }) { const npmScope = frameworkName === "SAPUI5" ? "@sapui5" : "@openui5"; @@ -787,8 +787,8 @@ test.serial("ui5Framework translator should not try to install anything when no t.is(pacote.manifest.callCount, 0, "No manifest should be requested"); }); -test.serial("ui5Framework helper should throw an error when framework version is not defined", async (t) => { - const {ui5Framework, projectGraphBuilder} = t.context; +test.serial("ui5Framework helper shouldn't throw when framework version and libraries are not provided", async (t) => { + const {ui5Framework, projectGraphBuilder, logStub} = t.context; const dependencyTree = { id: "test-id", @@ -809,9 +809,24 @@ test.serial("ui5Framework helper should throw an error when framework version is const provider = new DependencyTreeProvider({dependencyTree}); const projectGraph = await projectGraphBuilder(provider); - await t.throwsAsync(async () => { - await ui5Framework.enrichProjectGraph(projectGraph); - }, {message: `No framework version defined for root project test-project`}, "Correct error message"); + await ui5Framework.enrichProjectGraph(projectGraph); + + t.is(logStub.verbose.callCount, 5); + t.deepEqual(logStub.verbose.getCall(0).args, [ + "Configuration for module test-id has been supplied directly" + ]); + t.deepEqual(logStub.verbose.getCall(1).args, [ + "Module test-id contains project test-project" + ]); + t.deepEqual(logStub.verbose.getCall(2).args, [ + "Root project test-project qualified as application project for project graph" + ]); + t.deepEqual(logStub.verbose.getCall(3).args, [ + "Project test-project has no framework dependencies" + ]); + t.deepEqual(logStub.verbose.getCall(4).args, [ + "No SAPUI5 libraries referenced in project test-project or in any of its dependencies" + ]); }); test.serial( diff --git a/test/lib/graph/helpers/ui5Framework.js b/test/lib/graph/helpers/ui5Framework.js index b1237d153..981e65075 100644 --- a/test/lib/graph/helpers/ui5Framework.js +++ b/test/lib/graph/helpers/ui5Framework.js @@ -139,6 +139,34 @@ test.serial("enrichProjectGraph", async (t) => { ], "Traversed graph in correct order"); }); +test.serial("enrichProjectGraph: without framework configuration", async (t) => { + const {ui5Framework, log} = t.context; + const dependencyTree = { + id: "application.a", + version: "1.2.3", + path: applicationAPath, + configuration: { + specVersion: "2.0", + type: "application", + metadata: { + name: "application.a" + } + } + }; + + t.is(log.verbose.callCount, 0); + + const provider = new DependencyTreeProvider({dependencyTree}); + const projectGraph = await projectGraphBuilder(provider); + + await ui5Framework.enrichProjectGraph(projectGraph); + t.is(projectGraph.getSize(), 1, "Project graph should remain unchanged"); + t.is(log.verbose.callCount, 1); + t.deepEqual(log.verbose.getCall(0).args, [ + "Root project application.a has no framework configuration. Nothing to do here" + ]); +}); + test.serial("enrichProjectGraph: With versionOverride", async (t) => { const { sinon, ui5Framework, utils, @@ -192,8 +220,8 @@ test.serial("enrichProjectGraph: With versionOverride", async (t) => { }], "Sapui5Resolver#constructor should be called with expected args"); }); -test.serial("enrichProjectGraph should throw error when no framework version is provided", async (t) => { - const {ui5Framework} = t.context; +test.serial("enrichProjectGraph shouldn't throw when no framework version and no libraries are provided", async (t) => { + const {ui5Framework, log} = t.context; const dependencyTree = { id: "test-id", version: "1.2.3", @@ -213,9 +241,15 @@ test.serial("enrichProjectGraph should throw error when no framework version is const provider = new DependencyTreeProvider({dependencyTree}); const projectGraph = await projectGraphBuilder(provider); - await t.throwsAsync(async () => { - await ui5Framework.enrichProjectGraph(projectGraph); - }, {message: "No framework version defined for root project application.a"}); + await ui5Framework.enrichProjectGraph(projectGraph); + + t.is(log.verbose.callCount, 2); + t.deepEqual(log.verbose.getCall(0).args, [ + "Project application.a has no framework dependencies" + ]); + t.deepEqual(log.verbose.getCall(1).args, [ + "No SAPUI5 libraries referenced in project application.a or in any of its dependencies" + ]); }); test.serial("enrichProjectGraph should skip framework project without version", async (t) => { @@ -393,7 +427,7 @@ test.serial("enrichProjectGraph should resolve framework project " + t.is(projectGraph.getSize(), 2, "Project graph should remain unchanged"); t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once"); - t.is(getFrameworkLibrariesFromGraphStub.callCount, 1, "getFrameworkLibrariesFromGrap should be called once"); + t.is(getFrameworkLibrariesFromGraphStub.callCount, 1, "getFrameworkLibrariesFromGraph should be called once"); t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{ cwd: dependencyTree.path, version: "1.99.9", @@ -401,8 +435,48 @@ test.serial("enrichProjectGraph should resolve framework project " + }], "Sapui5Resolver#constructor should be called with expected args"); }); -test.serial("enrichProjectGraph should throw for framework project with dependency missing in graph", async (t) => { +test.serial("enrichProjectGraph should skip framework project when all dependencies are in graph", async (t) => { const {ui5Framework} = t.context; + const dependencyTree = { + id: "@sapui5/project", + version: "1.2.3", + path: applicationAPath, + configuration: { + specVersion: "2.0", + type: "application", + metadata: { + name: "application.a" + }, + framework: { + name: "SAPUI5", + libraries: [ + {name: "lib1"} + ] + } + }, + dependencies: [{ + id: "@openui5/lib1", + version: "1.2.3", + path: libraryEPath, + configuration: { + specVersion: "2.0", + type: "library", + metadata: { + name: "lib1" + } + } + }] + }; + + const provider = new DependencyTreeProvider({dependencyTree}); + const projectGraph = await projectGraphBuilder(provider); + + await ui5Framework.enrichProjectGraph(projectGraph); + t.is(projectGraph.getSize(), 2, "Project graph should remain unchanged"); +}); + +test.serial("enrichProjectGraph should throw for framework project with dependency missing in graph", async (t) => { + const {ui5Framework, Sapui5ResolverInstallStub} = t.context; const dependencyTree = { id: "@sapui5/project", version: "1.2.3", @@ -424,12 +498,15 @@ test.serial("enrichProjectGraph should throw for framework project with dependen } }; + const installError = new Error("Resolution of framework libraries failed with errors: TEST ERROR"); + + Sapui5ResolverInstallStub.rejects(installError); + const provider = new DependencyTreeProvider({dependencyTree}); const projectGraph = await projectGraphBuilder(provider); const err = await t.throwsAsync(ui5Framework.enrichProjectGraph(projectGraph)); - t.is(err.message, `Missing framework dependency lib1 for framework project application.a`, - "Threw with expected error message"); + t.is(err.message, installError.message); }); test.serial("enrichProjectGraph should throw for incorrect framework name", async (t) => { @@ -565,6 +642,120 @@ test.serial("enrichProjectGraph should throw error when projectGraph contains a }); }); +test.serial("enrichProjectGraph should use framework library metadata from workspace", async (t) => { + const {ui5Framework, utils, Sapui5ResolverStub, Sapui5ResolverInstallStub, sinon} = t.context; + const dependencyTree = { + id: "@sapui5/project", + version: "1.2.3", + path: applicationAPath, + configuration: { + specVersion: "2.0", + type: "application", + metadata: { + name: "application.a" + }, + framework: { + name: "SAPUI5", + version: "1.111.1", + libraries: [ + {name: "lib1"}, + {name: "lib2"} + ] + } + } + }; + + const workspace = { + getName: sinon.stub().resolves("default") + }; + + const workspaceFrameworkLibraryMetadata = {}; + const libraryMetadata = {}; + + sinon.stub(utils, "getWorkspaceFrameworkLibraryMetadata").resolves(workspaceFrameworkLibraryMetadata); + Sapui5ResolverInstallStub.resolves({libraryMetadata}); + + const addProjectToGraphStub = sinon.stub(); + sinon.stub(utils, "ProjectProcessor") + .callsFake(() => { + return { + addProjectToGraph: addProjectToGraphStub + }; + }); + + sinon.stub(utils, "declareFrameworkDependenciesInGraph").resolves(); + + const provider = new DependencyTreeProvider({dependencyTree}); + const projectGraph = await projectGraphBuilder(provider, {workspace}); + + await ui5Framework.enrichProjectGraph(projectGraph, {workspace}); + + t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once"); + t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{ + cwd: dependencyTree.path, + version: "1.111.1", + providedLibraryMetadata: workspaceFrameworkLibraryMetadata + }], "Sapui5Resolver#constructor should be called with expected args"); + t.is(Sapui5ResolverStub.getCall(0).args[0].providedLibraryMetadata, workspaceFrameworkLibraryMetadata); +}); + +test.serial("enrichProjectGraph should allow omitting framework version in case " + + "all framework libraries come from the workspace", async (t) => { + const {ui5Framework, utils, Sapui5ResolverStub, Sapui5ResolverInstallStub, sinon} = t.context; + const dependencyTree = { + id: "@sapui5/project", + version: "1.2.3", + path: applicationAPath, + configuration: { + specVersion: "2.0", + type: "application", + metadata: { + name: "application.a" + }, + framework: { + name: "SAPUI5", + libraries: [ + {name: "lib1"}, + {name: "lib2"} + ] + } + } + }; + + const workspace = { + getName: sinon.stub().resolves("default") + }; + + const workspaceFrameworkLibraryMetadata = {}; + const libraryMetadata = {}; + + sinon.stub(utils, "getWorkspaceFrameworkLibraryMetadata").resolves(workspaceFrameworkLibraryMetadata); + Sapui5ResolverInstallStub.resolves({libraryMetadata}); + + const addProjectToGraphStub = sinon.stub(); + sinon.stub(utils, "ProjectProcessor") + .callsFake(() => { + return { + addProjectToGraph: addProjectToGraphStub + }; + }); + + sinon.stub(utils, "declareFrameworkDependenciesInGraph").resolves(); + + const provider = new DependencyTreeProvider({dependencyTree}); + const projectGraph = await projectGraphBuilder(provider, {workspace}); + + await ui5Framework.enrichProjectGraph(projectGraph, {workspace}); + + t.is(Sapui5ResolverStub.callCount, 1, "Sapui5Resolver#constructor should be called once"); + t.deepEqual(Sapui5ResolverStub.getCall(0).args, [{ + cwd: dependencyTree.path, + version: undefined, + providedLibraryMetadata: workspaceFrameworkLibraryMetadata + }], "Sapui5Resolver#constructor should be called with expected args"); + t.is(Sapui5ResolverStub.getCall(0).args[0].providedLibraryMetadata, workspaceFrameworkLibraryMetadata); +}); + test.serial("utils.shouldIncludeDependency", (t) => { const {utils} = t.context; // root project dependency should always be included diff --git a/test/lib/ui5framework/AbstractResolver.js b/test/lib/ui5framework/AbstractResolver.js index 7b1da9df3..53b165002 100644 --- a/test/lib/ui5framework/AbstractResolver.js +++ b/test/lib/ui5framework/AbstractResolver.js @@ -46,13 +46,6 @@ test("AbstractResolver: constructor", (t) => { t.true(resolver instanceof AbstractResolver, "Constructor returns instance of abstract class"); }); -test("AbstractResolver: constructor requires 'version'", (t) => { - const {MyResolver} = t.context; - t.throws(() => { - new MyResolver({}); - }, {message: `AbstractResolver: Missing parameter "version"`}); -}); - test("AbstractResolver: Set absolute 'cwd'", (t) => { const {MyResolver} = t.context; const resolver = new MyResolver({ From c9ce0ffb506d6e946b5ba5c8284925daf821508d Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 29 Mar 2023 12:46:45 +0200 Subject: [PATCH 06/12] [INTERNAL] Add code comments --- lib/graph/helpers/ui5Framework.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index 5b253053a..e491bd3ea 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -347,6 +347,7 @@ export default { return projectGraph; } + // TODO: adopt for case without version log.info(`Using ${frameworkName} version: ${version}`); let providedLibraryMetadata; @@ -356,6 +357,8 @@ export default { }); } + // Note: version might be undefined here and the Resolver will throw an error when calling + // #install and it can't be resolved via the provided library metadata const resolver = new Resolver({cwd: rootProject.getRootPath(), version, providedLibraryMetadata}); let startTime; From c6e4c9f31375d337a0dc17e06815ef9ec4674a3f Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 29 Mar 2023 16:53:19 +0200 Subject: [PATCH 07/12] [INTERNAL] JSDoc adjustments --- lib/ui5Framework/AbstractResolver.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/ui5Framework/AbstractResolver.js b/lib/ui5Framework/AbstractResolver.js index 0fbf09dc2..d698970a3 100644 --- a/lib/ui5Framework/AbstractResolver.js +++ b/lib/ui5Framework/AbstractResolver.js @@ -29,10 +29,11 @@ const VERSION_RANGE_REGEXP = /^(0|[1-9]\d*)\.(0|[1-9]\d*)$/; class AbstractResolver { /** * @param {*} options options - * @param {string} options.version Framework version to use + * @param {string} [options.version] Framework version to use. * @param {string} [options.cwd=process.cwd()] Working directory to resolve configurations like .npmrc * @param {string} [options.ui5HomeDir="~/.ui5"] UI5 home directory location. This will be used to store packages, * metadata and configuration used by the resolvers. Relative to `process.cwd()` + * @param {string} [options.providedLibraryMetadata] TODO */ constructor({cwd, version, ui5HomeDir, providedLibraryMetadata}) { if (new.target === AbstractResolver) { From 2fb9cb9a3a415711cf2d25e9368d037f4d0430eb Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 30 Mar 2023 15:28:27 +0200 Subject: [PATCH 08/12] [INTERNAL] Update JSDoc / tests --- lib/ui5Framework/AbstractResolver.js | 11 ++++++++--- test/lib/ui5framework/AbstractResolver.js | 9 +++++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/ui5Framework/AbstractResolver.js b/lib/ui5Framework/AbstractResolver.js index d698970a3..09b281e35 100644 --- a/lib/ui5Framework/AbstractResolver.js +++ b/lib/ui5Framework/AbstractResolver.js @@ -27,14 +27,20 @@ const VERSION_RANGE_REGEXP = /^(0|[1-9]\d*)\.(0|[1-9]\d*)$/; * @hideconstructor */ class AbstractResolver { + /* eslint-disable max-len */ /** * @param {*} options options - * @param {string} [options.version] Framework version to use. + * @param {string} [options.version] Framework version to use. When omitted, all libraries need to be available + * via providedLibraryMetadata parameter. Otherwise an error is thrown. * @param {string} [options.cwd=process.cwd()] Working directory to resolve configurations like .npmrc * @param {string} [options.ui5HomeDir="~/.ui5"] UI5 home directory location. This will be used to store packages, * metadata and configuration used by the resolvers. Relative to `process.cwd()` - * @param {string} [options.providedLibraryMetadata] TODO + * @param {object.} [options.providedLibraryMetadata] + * Resolver skips installing listed libraries and uses the dependency information to resolve their dependencies. + * version can be omitted in case all libraries can be resolved via the providedLibraryMetadata. + * Otherwise an error is thrown. */ + /* eslint-enable max-len */ constructor({cwd, version, ui5HomeDir, providedLibraryMetadata}) { if (new.target === AbstractResolver) { throw new TypeError("Class 'AbstractResolver' is abstract"); @@ -157,7 +163,6 @@ class AbstractResolver { * Object containing all installed libraries with library name as key */ - /* eslint-enable max-len */ /** * Installs the provided libraries and their dependencies * diff --git a/test/lib/ui5framework/AbstractResolver.js b/test/lib/ui5framework/AbstractResolver.js index 53b165002..c4ec5a1ce 100644 --- a/test/lib/ui5framework/AbstractResolver.js +++ b/test/lib/ui5framework/AbstractResolver.js @@ -44,6 +44,15 @@ test("AbstractResolver: constructor", (t) => { }); t.true(resolver instanceof MyResolver, "Constructor returns instance of sub-class"); t.true(resolver instanceof AbstractResolver, "Constructor returns instance of abstract class"); + t.is(resolver._version, "1.75.0"); +}); + +test("AbstractResolver: constructor without version", (t) => { + const {MyResolver} = t.context; + const resolver = new MyResolver({ + cwd: "/test-project/" + }); + t.is(resolver._version, undefined); }); test("AbstractResolver: Set absolute 'cwd'", (t) => { From 9cba8afb99da4e07094cd4eef6c27ccd66626c53 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 30 Mar 2023 15:39:42 +0200 Subject: [PATCH 09/12] [INTERNAL] Use new APIs --- lib/graph/helpers/ui5Framework.js | 11 +++++------ .../graph/helpers/ui5Framework.integration.js | 19 ++++++++++++------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index e491bd3ea..175538b68 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -218,8 +218,7 @@ const utils = { let dependencies = []; let optionalDependencies = []; - // TODO: Add getter for ID - if (project.__id.startsWith("@sapui5/")) { + if (project.getId().startsWith("@sapui5/")) { project.getFrameworkDependencies().forEach((dependency) => { if (dependency.optional) { // Add optional dependency to optionalDependencies @@ -229,7 +228,7 @@ const utils = { dependencies.push(dependency.name); } }); - } else if (project.__id.startsWith("@openui5/")) { + } else if (project.getId().startsWith("@openui5/")) { const packageResource = await project.getRootReader().byPath("/package.json"); const packageInfo = JSON.parse(await packageResource.getString()); @@ -245,12 +244,12 @@ const utils = { }, async getWorkspaceFrameworkLibraryMetadata({workspace, projectGraph}) { const libraryMetadata = Object.create(null); - const {projectNameMap} = await workspace._getResolvedModules(); // TODO: private API usage - for (const ui5Module of projectNameMap.values()) { + const ui5Modules = await workspace.getModules(); + for (const ui5Module of ui5Modules) { const {project} = await ui5Module.getSpecifications(); if (project.isFrameworkProject?.() && !projectGraph.getProject(project.getName())) { const metadata = libraryMetadata[project.getName()] = Object.create(null); - metadata.id = project.__id; + metadata.id = project.getId(); metadata.path = project.getRootPath(); metadata.version = project.getVersion(); const {dependencies, optionalDependencies} = await utils.getFrameworkLibraryDependencies(project); diff --git a/test/lib/graph/helpers/ui5Framework.integration.js b/test/lib/graph/helpers/ui5Framework.integration.js index 9e2cac146..7cff9788a 100644 --- a/test/lib/graph/helpers/ui5Framework.integration.js +++ b/test/lib/graph/helpers/ui5Framework.integration.js @@ -423,7 +423,7 @@ function defineTest(testName, { getVersion: sinon.stub().returns("1.76.0-SNAPSHOT"), getRootPath: sinon.stub().returns(path.join(fakeBaseDir, "workspace", libName)), isFrameworkProject: sinon.stub().returns(true), - __id: libraryDistMetadata.npmPackageName, + getId: sinon.stub().returns(libraryDistMetadata.npmPackageName), getRootReader: sinon.stub().returns({ byPath: sinon.stub().resolves({ getString: sinon.stub().resolves(JSON.stringify({dependencies: {}})) @@ -448,17 +448,22 @@ function defineTest(testName, { getPath: sinon.stub().returns(path.join(fakeBaseDir, "workspace", libName)), }; projectNameMap.set(libName, module); - moduleIdMap.set(libraryDistMetadata.npmPackageName); + moduleIdMap.set(libraryDistMetadata.npmPackageName, module); }); - const getModuleByProjectName = sinon.stub().callsFake((projectName) => projectNameMap.get(projectName)); + const getModuleByProjectName = sinon.stub().callsFake( + async (projectName) => projectNameMap.get(projectName) + ); + const getModules = sinon.stub().callsFake( + async () => { + const sortedMap = new Map([...moduleIdMap].sort((a, b) => String(a[0]).localeCompare(b[0]))); + return Array.from(sortedMap.values()); + } + ); const workspace = { getName: sinon.stub().returns("test"), - _getResolvedModules: sinon.stub().resolves({ - projectNameMap, - moduleIdMap - }), + getModules, getModuleByProjectName }; From 7da8c82630ae9fe607ad44be39171b6e8ea09bb8 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Thu, 30 Mar 2023 16:32:38 +0200 Subject: [PATCH 10/12] [INTERNAL] More tests --- lib/graph/helpers/ui5Framework.js | 2 +- test/lib/graph/helpers/ui5Framework.js | 87 ++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index 175538b68..88a967a73 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -247,7 +247,7 @@ const utils = { const ui5Modules = await workspace.getModules(); for (const ui5Module of ui5Modules) { const {project} = await ui5Module.getSpecifications(); - if (project.isFrameworkProject?.() && !projectGraph.getProject(project.getName())) { + if (project?.isFrameworkProject?.() && !projectGraph.getProject(project.getName())) { const metadata = libraryMetadata[project.getName()] = Object.create(null); metadata.id = project.getId(); metadata.path = project.getRootPath(); diff --git a/test/lib/graph/helpers/ui5Framework.js b/test/lib/graph/helpers/ui5Framework.js index 981e65075..c32dad6a3 100644 --- a/test/lib/graph/helpers/ui5Framework.js +++ b/test/lib/graph/helpers/ui5Framework.js @@ -1297,6 +1297,93 @@ test("utils.checkForDuplicateFrameworkProjects: Two duplicates", (t) => { }); }); +test.skip("utils.getFrameworkLibraryDependencies: ", async (t) => { + const {utils, sinon} = t.context; + + const project = {}; + + const result = await utils.getFrameworkLibraryDependencies(project); +}); + +test("utils.getWorkspaceFrameworkLibraryMetadata: No workspace modules", async (t) => { + const {utils, sinon} = t.context; + + const workspace = { + getModules: sinon.stub().resolves([]) + }; + + const libraryMetadata = await utils.getWorkspaceFrameworkLibraryMetadata({workspace, projectGraph: {}}); + + t.deepEqual(libraryMetadata, {}); +}); + +test("utils.getWorkspaceFrameworkLibraryMetadata: With workspace modules", async (t) => { + const {utils, sinon} = t.context; + + const workspace = { + getModules: sinon.stub().resolves([ + { + // Extensions don't have projects, should be ignored + getSpecifications: sinon.stub().resolves({ + project: null + }) + }, + { + getSpecifications: sinon.stub().resolves({ + project: { + // some types don't have a "isFrameworkProject" method + } + }) + }, + { + getSpecifications: sinon.stub().resolves({ + project: { + isFrameworkProject: sinon.stub().returns(false) + } + }) + }, + { + getSpecifications: sinon.stub().resolves({ + project: { + isFrameworkProject: sinon.stub().returns(true), + getName: sinon.stub().returns("framework.project.within.projectGraph") + } + }) + }, + { + getSpecifications: sinon.stub().resolves({ + project: { + isFrameworkProject: sinon.stub().returns(true), + getName: sinon.stub().returns("framework.project.not.within.projectGraph"), + getId: sinon.stub().returns("id"), + getRootPath: sinon.stub().returns("/rootPath"), + getVersion: sinon.stub().returns("1.0.0"), + } + }) + } + ]) + }; + + const getProject = sinon.stub().returns(undefined); + getProject.withArgs("framework.project.within.projectGraph").returns({}); + getProject.withArgs("framework.project.not.within.projectGraph").returns(undefined); + const projectGraph = { + getProject + }; + + const libraryMetadata = await utils.getWorkspaceFrameworkLibraryMetadata({workspace, projectGraph}); + + t.deepEqual(libraryMetadata, { + "framework.project.not.within.projectGraph": { + dependencies: [], + id: "id", + optionalDependencies: [], + path: "/rootPath", + version: "1.0.0" + }, + }); +}); + test.serial("ProjectProcessor: Add project to graph", async (t) => { const {sinon} = t.context; const {ProjectProcessor} = t.context.utils; From 0425ff9fb34f2dfa7dd3be03856b1b95dfbb45d8 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 31 Mar 2023 12:18:25 +0200 Subject: [PATCH 11/12] [INTERNAL] More tests - continued --- lib/graph/helpers/ui5Framework.js | 3 + test/lib/graph/helpers/ui5Framework.js | 171 +++++++++++++++++++++++-- 2 files changed, 162 insertions(+), 12 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index 88a967a73..34efb99f3 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -247,6 +247,9 @@ const utils = { const ui5Modules = await workspace.getModules(); for (const ui5Module of ui5Modules) { const {project} = await ui5Module.getSpecifications(); + // Only framework projects that are not already part of the projectGraph should be handled. + // Otherwise they would be available twice which is checked + // after installing via checkForDuplicateFrameworkProjects if (project?.isFrameworkProject?.() && !projectGraph.getProject(project.getName())) { const metadata = libraryMetadata[project.getName()] = Object.create(null); metadata.id = project.getId(); diff --git a/test/lib/graph/helpers/ui5Framework.js b/test/lib/graph/helpers/ui5Framework.js index c32dad6a3..863646fe4 100644 --- a/test/lib/graph/helpers/ui5Framework.js +++ b/test/lib/graph/helpers/ui5Framework.js @@ -1297,12 +1297,90 @@ test("utils.checkForDuplicateFrameworkProjects: Two duplicates", (t) => { }); }); -test.skip("utils.getFrameworkLibraryDependencies: ", async (t) => { +test("utils.getFrameworkLibraryDependencies: OpenUI5 library", async (t) => { const {utils, sinon} = t.context; - const project = {}; + const project = { + getId: sinon.stub().returns("@openui5/sap.ui.lib1"), + getRootReader: sinon.stub().returns({ + byPath: sinon.stub().withArgs("/package.json").resolves({ + getString: sinon.stub().resolves(JSON.stringify({ + dependencies: { + "@openui5/sap.ui.lib2": "*" + }, + devDependencies: { + "@openui5/themelib_fancy": "*" + } + })) + }) + }), + }; + + const result = await utils.getFrameworkLibraryDependencies(project); + t.deepEqual(result, { + dependencies: ["sap.ui.lib2"], + optionalDependencies: ["themelib_fancy"] + }); +}); + +test("utils.getFrameworkLibraryDependencies: SAPUI5 library", async (t) => { + const {utils, sinon} = t.context; + + const project = { + getId: sinon.stub().returns("@sapui5/sap.ui.lib1"), + getFrameworkDependencies: sinon.stub().returns([ + { + name: "sap.ui.lib2" + }, + { + name: "themelib_fancy", + optional: true + }, + { + name: "sap.ui.lib3", + development: true + } + ]) + }; + + const result = await utils.getFrameworkLibraryDependencies(project); + t.deepEqual(result, { + dependencies: ["sap.ui.lib2"], + optionalDependencies: ["themelib_fancy"] + }); +}); + +test("utils.getFrameworkLibraryDependencies: OpenUI5 library - no dependencies", async (t) => { + const {utils, sinon} = t.context; + + const project = { + getId: sinon.stub().returns("@openui5/sap.ui.lib1"), + getRootReader: sinon.stub().returns({ + byPath: sinon.stub().withArgs("/package.json").resolves({ + getString: sinon.stub().resolves(JSON.stringify({})) + }) + }), + }; + + const result = await utils.getFrameworkLibraryDependencies(project); + t.deepEqual(result, { + dependencies: [], + optionalDependencies: [] + }); +}); + +test("utils.getFrameworkLibraryDependencies: No framework library", async (t) => { + const {utils, sinon} = t.context; + + const project = { + getId: sinon.stub().returns("foo") + }; const result = await utils.getFrameworkLibraryDependencies(project); + t.deepEqual(result, { + dependencies: [], + optionalDependencies: [] + }); }); test("utils.getWorkspaceFrameworkLibraryMetadata: No workspace modules", async (t) => { @@ -1346,7 +1424,22 @@ test("utils.getWorkspaceFrameworkLibraryMetadata: With workspace modules", async getSpecifications: sinon.stub().resolves({ project: { isFrameworkProject: sinon.stub().returns(true), - getName: sinon.stub().returns("framework.project.within.projectGraph") + getName: sinon.stub().returns("sap.ui.lib1"), + getId: sinon.stub().returns("@openui5/sap.ui.lib1"), + getRootPath: sinon.stub().returns("/rootPath"), + getRootReader: sinon.stub().returns({ + byPath: sinon.stub().withArgs("/package.json").resolves({ + getString: sinon.stub().resolves(JSON.stringify({ + dependencies: { + "@openui5/sap.ui.lib2": "*" + }, + devDependencies: { + "@openui5/themelib_fancy": "*" + } + })) + }) + }), + getVersion: sinon.stub().returns("1.0.0"), } }) }, @@ -1354,19 +1447,31 @@ test("utils.getWorkspaceFrameworkLibraryMetadata: With workspace modules", async getSpecifications: sinon.stub().resolves({ project: { isFrameworkProject: sinon.stub().returns(true), - getName: sinon.stub().returns("framework.project.not.within.projectGraph"), - getId: sinon.stub().returns("id"), + getName: sinon.stub().returns("sap.ui.lib3"), + getId: sinon.stub().returns("@sapui5/sap.ui.lib3"), getRootPath: sinon.stub().returns("/rootPath"), getVersion: sinon.stub().returns("1.0.0"), + getFrameworkDependencies: sinon.stub().returns([ + { + name: "sap.ui.lib4" + }, + { + name: "sap.ui.lib5", + optional: true + }, + { + name: "sap.ui.lib6", + development: true + } + ]) } }) } ]) }; - const getProject = sinon.stub().returns(undefined); - getProject.withArgs("framework.project.within.projectGraph").returns({}); - getProject.withArgs("framework.project.not.within.projectGraph").returns(undefined); + const getProject = sinon.stub(); + getProject.withArgs("sap.ui.lib1").returns(undefined); const projectGraph = { getProject }; @@ -1374,16 +1479,58 @@ test("utils.getWorkspaceFrameworkLibraryMetadata: With workspace modules", async const libraryMetadata = await utils.getWorkspaceFrameworkLibraryMetadata({workspace, projectGraph}); t.deepEqual(libraryMetadata, { - "framework.project.not.within.projectGraph": { - dependencies: [], - id: "id", - optionalDependencies: [], + "sap.ui.lib1": { + dependencies: [ + "sap.ui.lib2" + ], + id: "@openui5/sap.ui.lib1", + optionalDependencies: [ + "themelib_fancy" + ], + path: "/rootPath", + version: "1.0.0" + }, + "sap.ui.lib3": { + dependencies: [ + "sap.ui.lib4" + ], + id: "@sapui5/sap.ui.lib3", + optionalDependencies: [ + "sap.ui.lib5" + ], path: "/rootPath", version: "1.0.0" }, }); }); +test("utils.getWorkspaceFrameworkLibraryMetadata: With workspace module within projectGraph", async (t) => { + const {utils, sinon} = t.context; + + const workspace = { + getModules: sinon.stub().resolves([ + { + getSpecifications: sinon.stub().resolves({ + project: { + isFrameworkProject: sinon.stub().returns(true), + getName: sinon.stub().returns("sap.ui.lib1") + } + }) + } + ]) + }; + + const getProject = sinon.stub(); + getProject.withArgs("sap.ui.lib1").returns({}); + const projectGraph = { + getProject + }; + + const libraryMetadata = await utils.getWorkspaceFrameworkLibraryMetadata({workspace, projectGraph}); + + t.deepEqual(libraryMetadata, {}); +}); + test.serial("ProjectProcessor: Add project to graph", async (t) => { const {sinon} = t.context; const {ProjectProcessor} = t.context.utils; From 563e69aaeabc6f4b794ae3ddb55f4b22eee649fd Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Fri, 31 Mar 2023 14:22:44 +0200 Subject: [PATCH 12/12] [INTERNAL] AbstractResolver tests --- lib/graph/helpers/ui5Framework.js | 5 +- test/lib/ui5framework/AbstractResolver.js | 201 +++++++++++++++++++++- 2 files changed, 202 insertions(+), 4 deletions(-) diff --git a/lib/graph/helpers/ui5Framework.js b/lib/graph/helpers/ui5Framework.js index 34efb99f3..6df9258b0 100644 --- a/lib/graph/helpers/ui5Framework.js +++ b/lib/graph/helpers/ui5Framework.js @@ -349,8 +349,9 @@ export default { return projectGraph; } - // TODO: adopt for case without version - log.info(`Using ${frameworkName} version: ${version}`); + if (version) { + log.info(`Using ${frameworkName} version: ${version}`); + } let providedLibraryMetadata; if (workspace) { diff --git a/test/lib/ui5framework/AbstractResolver.js b/test/lib/ui5framework/AbstractResolver.js index c4ec5a1ce..a89d48ed0 100644 --- a/test/lib/ui5framework/AbstractResolver.js +++ b/test/lib/ui5framework/AbstractResolver.js @@ -38,13 +38,16 @@ test("AbstractResolver: abstract constructor should throw", async (t) => { test("AbstractResolver: constructor", (t) => { const {MyResolver, AbstractResolver} = t.context; + const providedLibraryMetadata = {"test": "data"}; const resolver = new MyResolver({ cwd: "/test-project/", - version: "1.75.0" + version: "1.75.0", + providedLibraryMetadata }); t.true(resolver instanceof MyResolver, "Constructor returns instance of sub-class"); t.true(resolver instanceof AbstractResolver, "Constructor returns instance of abstract class"); t.is(resolver._version, "1.75.0"); + t.is(resolver._providedLibraryMetadata, providedLibraryMetadata); }); test("AbstractResolver: constructor without version", (t) => { @@ -206,9 +209,160 @@ test("AbstractResolver: install", async (t) => { install: Promise.resolve({pkgPath: "/foo/sap.ui.lib4"}) }); - await resolver.install(["sap.ui.lib1", "sap.ui.lib2", "sap.ui.lib4"]); + const result = await resolver.install(["sap.ui.lib1", "sap.ui.lib2", "sap.ui.lib4"]); t.is(handleLibraryStub.callCount, 4, "Each library should be handled once"); + t.deepEqual(result, { + libraryMetadata: { + "sap.ui.lib1": { + dependencies: [], + npmPackageName: "@openui5/sap.ui.lib1", + optionalDependencies: [], + path: "/foo/sap.ui.lib1", + version: "1.75.0", + }, + "sap.ui.lib2": { + dependencies: [ + "sap.ui.lib3", + ], + npmPackageName: "@openui5/sap.ui.lib2", + optionalDependencies: [], + path: "/foo/sap.ui.lib2", + version: "1.75.0", + }, + "sap.ui.lib3": { + dependencies: [], + npmPackageName: "@openui5/sap.ui.lib3", + optionalDependencies: [ + "sap.ui.lib4", + ], + path: "/foo/sap.ui.lib3", + version: "1.75.0", + }, + "sap.ui.lib4": { + dependencies: [ + "sap.ui.lib1", + ], + npmPackageName: "@openui5/sap.ui.lib4", + optionalDependencies: [], + path: "/foo/sap.ui.lib4", + version: "1.75.0", + }, + } + }); +}); + +test("AbstractResolver: install (with providedLibraryMetadata)", async (t) => { + const {MyResolver} = t.context; + const resolver = new MyResolver({ + cwd: "/test-project/", + version: "1.75.0", + providedLibraryMetadata: { + "sap.ui.lib1": { + "npmPackageName": "@openui5/sap.ui.lib1", + "version": "1.75.0-workspace", + "dependencies": [ + "sap.ui.lib3" + ], + "optionalDependencies": [], + "path": "/workspace/sap.ui.lib1" + }, + "sap.ui.lib4": { + "npmPackageName": "@openui5/sap.ui.lib4", + "version": "1.75.0-workspace", + "dependencies": [ + "sap.ui.lib5" + ], + "optionalDependencies": [], + "path": "/workspace/sap.ui.lib4" + }, + "sap.ui.lib5": { + "npmPackageName": "@openui5/sap.ui.lib5", + "version": "1.75.0-workspace", + "dependencies": [], + "optionalDependencies": [], + "path": "/workspace/sap.ui.lib5" + }, + } + }); + + const metadata = { + libraries: { + "sap.ui.lib2": { + "npmPackageName": "@openui5/sap.ui.lib2", + "version": "1.75.0", + "dependencies": [], + "optionalDependencies": [] + }, + "sap.ui.lib3": { + "npmPackageName": "@openui5/sap.ui.lib3", + "version": "1.75.0", + "dependencies": [ + "sap.ui.lib4" + ], + "optionalDependencies": [] + }, + } + }; + + const handleLibraryStub = sinon.stub(resolver, "handleLibrary"); + handleLibraryStub + .callsFake(async (libraryName) => { + throw new Error(`Unknown handleLibrary call: ${libraryName}`); + }) + .withArgs("sap.ui.lib2").resolves({ + metadata: Promise.resolve(metadata.libraries["sap.ui.lib2"]), + install: Promise.resolve({pkgPath: "/foo/sap.ui.lib2"}) + }) + .withArgs("sap.ui.lib3").resolves({ + metadata: Promise.resolve(metadata.libraries["sap.ui.lib3"]), + install: Promise.resolve({pkgPath: "/foo/sap.ui.lib3"}) + }); + + const result = await resolver.install(["sap.ui.lib1", "sap.ui.lib2"]); + + t.is(handleLibraryStub.callCount, 2, "Each library not part of providedLibraryMetadata should be handled once"); + t.deepEqual(result, { + libraryMetadata: { + "sap.ui.lib1": { + dependencies: ["sap.ui.lib3"], + npmPackageName: "@openui5/sap.ui.lib1", + optionalDependencies: [], + path: "/workspace/sap.ui.lib1", + version: "1.75.0-workspace", + }, + "sap.ui.lib2": { + dependencies: [], + npmPackageName: "@openui5/sap.ui.lib2", + optionalDependencies: [], + path: "/foo/sap.ui.lib2", + version: "1.75.0", + }, + "sap.ui.lib3": { + dependencies: ["sap.ui.lib4",], + npmPackageName: "@openui5/sap.ui.lib3", + optionalDependencies: [], + path: "/foo/sap.ui.lib3", + version: "1.75.0", + }, + "sap.ui.lib4": { + dependencies: [ + "sap.ui.lib5", + ], + npmPackageName: "@openui5/sap.ui.lib4", + optionalDependencies: [], + path: "/workspace/sap.ui.lib4", + version: "1.75.0-workspace", + }, + "sap.ui.lib5": { + dependencies: [], + npmPackageName: "@openui5/sap.ui.lib5", + optionalDependencies: [], + path: "/workspace/sap.ui.lib5", + version: "1.75.0-workspace", + }, + } + }); }); test("AbstractResolver: install error handling (rejection of metadata/install)", async (t) => { @@ -337,6 +491,49 @@ Failed to resolve library sap.ui.lib2: Error within handleLibrary: sap.ui.lib2`} t.is(handleLibraryStub.callCount, 2, "Each library should be handled once"); }); +test("AbstractResolver: install error handling " + +"(no version, no providedLibraryMetadata)", async (t) => { + const {MyResolver} = t.context; + const resolver = new MyResolver({ + cwd: "/test-project/", + }); + + const handleLibraryStub = sinon.stub(resolver, "handleLibrary"); + + await t.throwsAsync(resolver.install(["sap.ui.lib1", "sap.ui.lib2"]), { + message: `Resolution of framework libraries failed with errors: +Failed to resolve library sap.ui.lib1: Unable to install library sap.ui.lib1. No framework version provided. +Failed to resolve library sap.ui.lib2: Unable to install library sap.ui.lib2. No framework version provided.` + }); + + t.is(handleLibraryStub.callCount, 0, "Handle library should not be called when no version is available"); +}); + +test("AbstractResolver: install error handling " + +"(no version, one lib not part of providedLibraryMetadata)", async (t) => { + const {MyResolver} = t.context; + const resolver = new MyResolver({ + cwd: "/test-project/", + providedLibraryMetadata: { + "sap.ui.lib1": { + "npmPackageName": "@openui5/sap.ui.lib1", + "version": "1.75.0-SNAPSHOT", + "dependencies": [], + "optionalDependencies": [] + } + } + }); + + const handleLibraryStub = sinon.stub(resolver, "handleLibrary"); + + await t.throwsAsync(resolver.install(["sap.ui.lib1", "sap.ui.lib2"]), { + message: `Resolution of framework libraries failed with errors: +Failed to resolve library sap.ui.lib2: Unable to install library sap.ui.lib2. No framework version provided.` + }); + + t.is(handleLibraryStub.callCount, 0, "Handle library should not be called when no version is available"); +}); + test("AbstractResolver: static fetchAllVersions should throw an Error when not implemented", async (t) => { const {AbstractResolver} = t.context; await t.throwsAsync(async () => {