Skip to content

Commit

Permalink
[FIX] ui5Framework.mergeTrees: Do not process the same project multip…
Browse files Browse the repository at this point in the history 
…le 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
  • Loading branch information
@RandomByte
RandomByte committed Apr 1, 2020
1 parent 19c2403 commit 1377ec2
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 63 deletions.
10 changes: 10 additions & 0 deletions lib/translators/ui5Framework.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,18 @@ module.exports = {
log.verbose(`Merging framework tree into project tree "${projectTree.metadata.name}"`);

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

project.dependencies = project.dependencies.filter((depProject) => {
if (utils.isFrameworkProject(depProject)) {
Expand Down
139 changes: 76 additions & 63 deletions test/lib/translators/ui5Framework.integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ test.beforeEach((t) => {
sinon.stub(lockfile, "lock").yieldsAsync();
sinon.stub(lockfile, "unlock").yieldsAsync();

const testLogger = logger.getLogger();
sinon.stub(logger, "getLogger").returns(testLogger);
t.context.logInfoSpy = sinon.spy(testLogger, "info");

mock("mkdirp", sinon.stub().resolves());

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

const testDependency = {
id: "test-dependency-id",
version: "4.5.6",
path: path.join(fakeBaseDir, "project-test-dependency"),
dependencies: []
};
const translatorTree = {
id: "test-application-id",
version: "1.2.3",
path: path.join(fakeBaseDir, "project-test-application"),
dependencies: [
{
id: "test-dependency-id",
version: "4.5.6",
path: path.join(fakeBaseDir, "project-test-dependency"),
dependencies: []
},
testDependency,
{
id: "test-dependency-no-framework-id",
version: "7.8.9",
path: path.join(fakeBaseDir, "project-test-dependency-no-framework"),
dependencies: []
dependencies: [
testDependency
]
},
{
id: "test-dependency-framework-old-spec-version-id",
Expand Down Expand Up @@ -377,96 +384,98 @@ function defineTest(testName, {
.resolves(distributionMetadata);
}

const expectedTree = project({
_level: 0,
name: "test-application",
version: "1.2.3",
type: "application",
const testDependencyProject = project({
_level: 1,
name: "test-dependency",
version: "4.5.6",
type: "library",
framework: {
version: "1.99.0",
name: frameworkName,
version: "1.75.0",
libraries: [
{
name: "sap.ui.lib1"
},
{
name: "sap.ui.lib4",
name: "sap.ui.lib2"
},
{
name: "sap.ui.lib5",
optional: true
},
{
name: "sap.ui.lib8",
name: "sap.ui.lib6",
development: true
},
{
name: "sap.ui.lib8",
optional: true
}
]
},
dependencies: [
project({
frameworkProject({
_level: 1,
name: "test-dependency",
version: "4.5.6",
type: "library",
framework: {
version: "1.99.0",
name: frameworkName,
libraries: [
{
name: "sap.ui.lib1"
},
{
name: "sap.ui.lib2"
},
{
name: "sap.ui.lib5",
optional: true
},
{
name: "sap.ui.lib6",
development: true
},
{
name: "sap.ui.lib8",
optional: true
}
]
},
name: "sap.ui.lib1",
}),
frameworkProject({
_level: 1,
name: "sap.ui.lib2",
dependencies: [
frameworkProject({
_level: 1,
name: "sap.ui.lib1",
}),
frameworkProject({
_level: 1,
name: "sap.ui.lib2",
_level: 2,
name: "sap.ui.lib3",
dependencies: [
frameworkProject({
_level: 2,
name: "sap.ui.lib3",
name: "sap.ui.lib4",
_level: 1,
dependencies: [
frameworkProject({
name: "sap.ui.lib4",
_level: 1,
dependencies: [
frameworkProject({
_level: 1,
name: "sap.ui.lib1"
})
]
name: "sap.ui.lib1"
})
]
})
]
}),
frameworkProject({
_level: 1,
name: "sap.ui.lib8",
})
]
}),
frameworkProject({
_level: 1,
name: "sap.ui.lib8",
})
]
});
const expectedTree = project({
_level: 0,
name: "test-application",
version: "1.2.3",
type: "application",
framework: {
name: frameworkName,
version: "1.75.0",
libraries: [
{
name: "sap.ui.lib1"
},
{
name: "sap.ui.lib4",
optional: true
},
{
name: "sap.ui.lib8",
development: true
}
]
},
dependencies: [
testDependencyProject,
project({
_level: 1,
name: "test-dependency-no-framework",
version: "7.8.9",
type: "library"
type: "library",
dependencies: [testDependencyProject]
}),
project({
_level: 1,
Expand Down Expand Up @@ -506,6 +515,10 @@ function defineTest(testName, {
const tree = await normalizer.generateProjectTree();

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

Expand Down

0 comments on commit 1377ec2

Please sign in to comment.