Skip to content

Commit 1377ec2

Browse files
committed
[FIX] ui5Framework.mergeTrees: Do not process the same project multiple times
Since projects are deduped by the projectPreprocessor, repeated processing of a project that appears multiple times in the project tree is done on the same object. This leads to issues like the isFrameworkProject detecting previously added framework libraries as ... "already added UI5 framework libraries", thus producing incorrect log messages
1 parent 19c2403 commit 1377ec2

File tree

2 files changed

+86
-63
lines changed

2 files changed

+86
-63
lines changed

lib/translators/ui5Framework.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,18 @@ module.exports = {
212212
log.verbose(`Merging framework tree into project tree "${projectTree.metadata.name}"`);
213213

214214
const queue = [projectTree];
215+
const processedProjects = [];
215216
while (queue.length) {
216217
const project = queue.shift();
218+
if (processedProjects.includes(project.id)) {
219+
// projectTree must be duplicate free. A second occurrence of the same project
220+
// is always the same object. Therefore a single processing needs to be ensured.
221+
// Otherwise the isFrameworkProject check would detect framework dependencies added
222+
// at an earlier processing of the project and yield incorrect logging.
223+
log.verbose(`Project ${project.metadata.name} (${project.id}) has already been processed`);
224+
return;
225+
}
226+
processedProjects.push(project.id);
217227

218228
project.dependencies = project.dependencies.filter((depProject) => {
219229
if (utils.isFrameworkProject(depProject)) {

test/lib/translators/ui5Framework.integration.js

Lines changed: 76 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ test.beforeEach((t) => {
3434
sinon.stub(lockfile, "lock").yieldsAsync();
3535
sinon.stub(lockfile, "unlock").yieldsAsync();
3636

37+
const testLogger = logger.getLogger();
38+
sinon.stub(logger, "getLogger").returns(testLogger);
39+
t.context.logInfoSpy = sinon.spy(testLogger, "info");
40+
3741
mock("mkdirp", sinon.stub().resolves());
3842

3943
// Re-require to ensure that mocked modules are used
@@ -149,22 +153,25 @@ function defineTest(testName, {
149153
logger.setLevel("verbose");
150154
}
151155

156+
const testDependency = {
157+
id: "test-dependency-id",
158+
version: "4.5.6",
159+
path: path.join(fakeBaseDir, "project-test-dependency"),
160+
dependencies: []
161+
};
152162
const translatorTree = {
153163
id: "test-application-id",
154164
version: "1.2.3",
155165
path: path.join(fakeBaseDir, "project-test-application"),
156166
dependencies: [
157-
{
158-
id: "test-dependency-id",
159-
version: "4.5.6",
160-
path: path.join(fakeBaseDir, "project-test-dependency"),
161-
dependencies: []
162-
},
167+
testDependency,
163168
{
164169
id: "test-dependency-no-framework-id",
165170
version: "7.8.9",
166171
path: path.join(fakeBaseDir, "project-test-dependency-no-framework"),
167-
dependencies: []
172+
dependencies: [
173+
testDependency
174+
]
168175
},
169176
{
170177
id: "test-dependency-framework-old-spec-version-id",
@@ -377,96 +384,98 @@ function defineTest(testName, {
377384
.resolves(distributionMetadata);
378385
}
379386

380-
const expectedTree = project({
381-
_level: 0,
382-
name: "test-application",
383-
version: "1.2.3",
384-
type: "application",
387+
const testDependencyProject = project({
388+
_level: 1,
389+
name: "test-dependency",
390+
version: "4.5.6",
391+
type: "library",
385392
framework: {
393+
version: "1.99.0",
386394
name: frameworkName,
387-
version: "1.75.0",
388395
libraries: [
389396
{
390397
name: "sap.ui.lib1"
391398
},
392399
{
393-
name: "sap.ui.lib4",
400+
name: "sap.ui.lib2"
401+
},
402+
{
403+
name: "sap.ui.lib5",
394404
optional: true
395405
},
396406
{
397-
name: "sap.ui.lib8",
407+
name: "sap.ui.lib6",
398408
development: true
409+
},
410+
{
411+
name: "sap.ui.lib8",
412+
optional: true
399413
}
400414
]
401415
},
402416
dependencies: [
403-
project({
417+
frameworkProject({
404418
_level: 1,
405-
name: "test-dependency",
406-
version: "4.5.6",
407-
type: "library",
408-
framework: {
409-
version: "1.99.0",
410-
name: frameworkName,
411-
libraries: [
412-
{
413-
name: "sap.ui.lib1"
414-
},
415-
{
416-
name: "sap.ui.lib2"
417-
},
418-
{
419-
name: "sap.ui.lib5",
420-
optional: true
421-
},
422-
{
423-
name: "sap.ui.lib6",
424-
development: true
425-
},
426-
{
427-
name: "sap.ui.lib8",
428-
optional: true
429-
}
430-
]
431-
},
419+
name: "sap.ui.lib1",
420+
}),
421+
frameworkProject({
422+
_level: 1,
423+
name: "sap.ui.lib2",
432424
dependencies: [
433425
frameworkProject({
434-
_level: 1,
435-
name: "sap.ui.lib1",
436-
}),
437-
frameworkProject({
438-
_level: 1,
439-
name: "sap.ui.lib2",
426+
_level: 2,
427+
name: "sap.ui.lib3",
440428
dependencies: [
441429
frameworkProject({
442-
_level: 2,
443-
name: "sap.ui.lib3",
430+
name: "sap.ui.lib4",
431+
_level: 1,
444432
dependencies: [
445433
frameworkProject({
446-
name: "sap.ui.lib4",
447434
_level: 1,
448-
dependencies: [
449-
frameworkProject({
450-
_level: 1,
451-
name: "sap.ui.lib1"
452-
})
453-
]
435+
name: "sap.ui.lib1"
454436
})
455437
]
456438
})
457439
]
458-
}),
459-
frameworkProject({
460-
_level: 1,
461-
name: "sap.ui.lib8",
462440
})
463441
]
464442
}),
443+
frameworkProject({
444+
_level: 1,
445+
name: "sap.ui.lib8",
446+
})
447+
]
448+
});
449+
const expectedTree = project({
450+
_level: 0,
451+
name: "test-application",
452+
version: "1.2.3",
453+
type: "application",
454+
framework: {
455+
name: frameworkName,
456+
version: "1.75.0",
457+
libraries: [
458+
{
459+
name: "sap.ui.lib1"
460+
},
461+
{
462+
name: "sap.ui.lib4",
463+
optional: true
464+
},
465+
{
466+
name: "sap.ui.lib8",
467+
development: true
468+
}
469+
]
470+
},
471+
dependencies: [
472+
testDependencyProject,
465473
project({
466474
_level: 1,
467475
name: "test-dependency-no-framework",
468476
version: "7.8.9",
469-
type: "library"
477+
type: "library",
478+
dependencies: [testDependencyProject]
470479
}),
471480
project({
472481
_level: 1,
@@ -506,6 +515,10 @@ function defineTest(testName, {
506515
const tree = await normalizer.generateProjectTree();
507516

508517
t.deepEqual(tree, expectedTree, "Returned tree should be correct");
518+
const frameworkLibAlreadyAddedInfoLogged = (t.context.logInfoSpy.getCalls()
519+
.map(($) => $.firstArg)
520+
.findIndex(($) => $.includes("defines a dependency to the UI5 framework library")) !== -1);
521+
t.false(frameworkLibAlreadyAddedInfoLogged, "No info regarding already added UI5 framework libraries logged");
509522
});
510523
}
511524

0 commit comments

Comments
 (0)