Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BREAKING] Make namespace mandatory for application and library projects #430

Merged
merged 9 commits into from
Mar 20, 2020
+101 −37
Merged

[BREAKING] Make namespace mandatory for application and library projects #430

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e619b32
[BREAKING] Make namespace mandatory for application and library projects
svbender Mar 16, 2020
9901f91
[BREAKING] Make namespace mandatory for application and library projects
svbender Mar 17, 2020
bb1bc76
Add Test for getNamespace call
tobiasso85 Mar 17, 2020
3c2f3a1
Add Test for getCopyright call
tobiasso85 Mar 17, 2020
d7c7ebf
re-added test for getNamespace
tobiasso85 Mar 17, 2020
7b8fc99
add test for ApplicationFormatter#getSourceBasePath
tobiasso85 Mar 17, 2020
7773b4e
add test for LibraryFormatter#getLibraryJsPath and getDotLibrary
tobiasso85 Mar 17, 2020
680dcd1
Updating unit test to check if getDotLibrary works when loading from …
svbender Mar 17, 2020
cc69b79
Updating unit test to check if getDotLibrary works when loading from …
svbender Mar 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions lib/types/application/ApplicationFormatter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,7 @@ class ApplicationFormatter extends AbstractUi5Formatter {
"/": project.resources.configuration.paths.webapp
};

try {
project.metadata.namespace = await this.getNamespace();
} catch (err) {
// Catch error because namespace is optional
// TODO 2.0: Make namespace mandatory and just let the error throw
log.warn(err.message);
}
project.metadata.namespace = await this.getNamespace();
}

/**
Expand Down
8 changes: 1 addition & 7 deletions lib/types/library/LibraryFormatter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,7 @@ class LibraryFormatter extends AbstractUi5Formatter {
"Either no setting was provided or the path not found.");
}

try {
project.metadata.namespace = await this.getNamespace();
} catch (err) {
// Catch error because namespace is optional
// TODO 2.0: Make namespace mandatory and just let the error throw
log.warn(err.message);
}
project.metadata.namespace = await this.getNamespace();

try {
project.metadata.copyright = await this.getCopyright();
Expand Down
27 changes: 17 additions & 10 deletions test/lib/types/application/ApplicationFormatter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,16 +130,24 @@ function createMockProject() {
};
}

test("getSourceBasePath: posix", async (t) => {
const myProject = clone(applicationBTree);
myProject.path = "my/pony";
const applicationFormatter = new ApplicationFormatter({project: myProject});

const sourceBasePath = applicationFormatter.getSourceBasePath(true);
t.is(sourceBasePath, "my/pony/webapp", "correct path");
});

test("format: No 'sap.app' configuration found", async (t) => {
const project = createMockProject();
const applicationFormatter = new ApplicationFormatter({project});
sinon.stub(applicationFormatter, "validate").resolves();
sinon.stub(applicationFormatter, "getManifest").resolves({content: {}, fsPath: {}});

await applicationFormatter.format();
t.deepEqual(project.resources.pathMappings["/"], "webapp", "path mappings is set");
t.falsy(project.metadata.namespace,
"namespace is falsy since getManifest resolves with an empty object");
const error = await t.throwsAsync(applicationFormatter.format());
t.deepEqual(error.message, "No \"sap.app\" ID configuration found in manifest.json of project projectName",
"Rejected with correct error message");
});

test("format: No application id in 'sap.app' configuration found", async (t) => {
Expand All @@ -148,10 +156,9 @@ test("format: No application id in 'sap.app' configuration found", async (t) =>
sinon.stub(applicationFormatter, "validate").resolves();
sinon.stub(applicationFormatter, "getManifest").resolves({content: {"sap.app": {}}});

await applicationFormatter.format();
const error = await t.throwsAsync(applicationFormatter.format());
t.deepEqual(error.message, "No \"sap.app\" ID configuration found in manifest.json of project projectName");
t.deepEqual(project.resources.pathMappings["/"], "webapp", "path mappings is set");
t.falsy(project.metadata.namespace,
"namespace is falsy since getManifest resolves with an empty object");
});

test("format: set namespace to id", async (t) => {
Expand Down Expand Up @@ -278,7 +285,7 @@ test("namespace: detect namespace from pom.xml via ${appId} from properties", as
myProject.resources.configuration.paths.webapp = "webapp-properties.appId";
const applicationFormatter = new ApplicationFormatter({project: myProject});

await applicationFormatter.format();
t.falsy(myProject.metadata.namespace,
"namespace is falsy since getManifest resolves with an empty object");
const error = await t.throwsAsync(applicationFormatter.format());
t.deepEqual(error.message, "Failed to resolve namespace of project application.h: \"${appId}\"" +
" couldn't be resolved from maven property \"appId\" of pom.xml of project application.h");
});
79 changes: 68 additions & 11 deletions test/lib/types/library/LibraryFormatter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,34 @@ test("format: copyright already configured", async (t) => {
t.deepEqual(myProject.metadata.copyright, libraryETree.metadata.copyright, "Copyright was not altered");
});

test.serial("format: copyright retrieval fails", async (t) => {
const myProject = clone(libraryETree);

const log = require("@ui5/logger");
const loggerInstance = log.getLogger("types:library:LibraryFormatter");

mock("@ui5/logger", {
getLogger: () => loggerInstance
});
mock.reRequire("@ui5/logger");
const loggerVerboseSpy = sinon.spy(loggerInstance, "verbose");

const LibraryFormatter = mock.reRequire("../../../../lib/types/library/LibraryFormatter");

const libraryFormatter = new LibraryFormatter({project: myProject});
sinon.stub(libraryFormatter, "validate").resolves();
sinon.stub(libraryFormatter, "getCopyright").rejects(Error("my-pony"));

await libraryFormatter.format();
t.deepEqual(myProject.metadata.copyright, libraryETree.metadata.copyright, "Copyright was not altered");


t.is(loggerVerboseSpy.callCount, 4, "calls to verbose");
t.is(loggerVerboseSpy.getCall(3).args[0], "my-pony", "message from rejection");

mock.stop("@ui5/logger");
});

test("format: formats correctly", async (t) => {
const myProject = clone(libraryETree);
myProject.metadata.copyright = undefined;
Expand Down Expand Up @@ -194,20 +222,22 @@ test.serial("format: namespace resolution fails", async (t) => {
});
mock.reRequire("@ui5/logger");
const loggerVerboseSpy = sinon.spy(loggerInstance, "verbose");
const loggerWarnSpy = sinon.spy(loggerInstance, "warn");

const LibraryFormatter = mock.reRequire("../../../../lib/types/library/LibraryFormatter");

const libraryFormatter = new LibraryFormatter({project: myProject});
sinon.stub(libraryFormatter, "validate").resolves();


await libraryFormatter.format();
const error = await t.throwsAsync(libraryFormatter.format());
t.deepEqual(error.message, "Failed to detect namespace or namespace is empty for project library.e." +
" Check verbose log for details.");

t.deepEqual(globbyStub.callCount, 3, "globby got called three times");
t.deepEqual(globbyStub.getCall(0).args[0], "**/manifest.json", "First glob is for manifest.json files");
t.deepEqual(globbyStub.getCall(1).args[0], "**/.library", "Second glob is for .library files");
t.deepEqual(globbyStub.getCall(2).args[0], "**/library.js", "Third glob for library.js files");
t.deepEqual(loggerVerboseSpy.callCount, 7, "7 calls to log.verbose should be done");
t.deepEqual(loggerVerboseSpy.callCount, 6, "7 calls to log.verbose should be done");
const logVerboseCalls = loggerVerboseSpy.getCalls().map((call) => call.args[0]);

t.true(logVerboseCalls.includes(
Expand All @@ -225,12 +255,6 @@ test.serial("format: namespace resolution fails", async (t) => {
"Could not find library.js file for project library.e"),
"should contain message for missing library.js");

t.deepEqual(loggerWarnSpy.callCount, 1, "1 calls to log.warn should be done");
const logWarnCalls = loggerWarnSpy.getCalls().map((call) => call.args[0]);
t.true(logWarnCalls.includes(
"Failed to detect namespace or namespace is empty for project library.e. Check verbose log for details."),
"should contain message for .library");

mock.stop("globby");
mock.stop("@ui5/logger");
});
Expand Down Expand Up @@ -497,8 +521,41 @@ test("getNamespace: from manifest.json with not matching file path", async (t) =
const err = await t.throwsAsync(libraryFormatter.getNamespace());

t.deepEqual(err.message, `Detected namespace "mani-pony" does not match detected directory structure ` +
`"different/namespace" for project library.e`,
"Rejected with correct error message");
`"different/namespace" for project library.e`, "Rejected with correct error message");
});

test.serial("getNamespace: from manifest.json without sap.app id", async (t) => {
const myProject = clone(libraryETree);

const log = require("@ui5/logger");
const loggerInstance = log.getLogger("types:library:LibraryFormatter");

mock("@ui5/logger", {
getLogger: () => loggerInstance
});
mock.reRequire("@ui5/logger");

const LibraryFormatter = mock.reRequire("../../../../lib/types/library/LibraryFormatter");

const libraryFormatter = new LibraryFormatter({project: myProject});
sinon.stub(libraryFormatter, "getManifest").resolves({
content: {
"sap.app": {
}
},
fsPath: path.normalize("/some/path/different/namespace/manifest.json") // normalize for windows
});
sinon.stub(libraryFormatter, "getSourceBasePath").returns("/some/path/");

const loggerSpy = sinon.spy(loggerInstance, "verbose");
const err = await t.throwsAsync(libraryFormatter.getNamespace());

t.deepEqual(err.message, `Failed to detect namespace or namespace is empty for project library.e. Check verbose log for details.`, "Rejected with correct error message");
t.is(loggerSpy.callCount, 4, "calls to verbose");


t.is(loggerSpy.getCall(0).args[0], "No \"sap.app\" ID configuration found in manifest.json of project library.e", "correct verbose message");
mock.stop("@ui5/logger");
});

test("getNamespace: from .library", async (t) => {
Expand Down