[INTERNAL] improve test coverage #88
Conversation
|
|
| }); | ||
| }).catch((error) => { | ||
| nodeServer.close(); | ||
| t.is( |
There was a problem hiding this comment.
I think we should use throws here.
If server does not throw in this test, no other asserts will explicitly make the test fail. It will still fail because no asserts are executed, but the moment someone adds another assertion, we can't be sure that the server actually threw.
| }); | ||
| }); | ||
| }).catch((error) => { | ||
| t.fail(error); |
There was a problem hiding this comment.
Not necessary, ava tests fail automatically if the returned promise rejects
| methods.forEach(function(method) { | ||
| res = { | ||
| status: function(status) { | ||
| t.is(status, 404, "Status should be 404"); |
There was a problem hiding this comment.
Rather use t.deepEqual. We do not care whether this is the same object but whether it is of equal value
t.is is like ==
t.deepEqual is like ===
(also see Object.is
| t.is(status, 404, "Status should be 404"); | ||
| return { | ||
| end: function(message) { | ||
| t.is(message, "Cannot " + method + " somePath", "Finished with error message."); |
| return server.serve(tree, { | ||
| port: 3335 | ||
| }).then((serveResult) => { | ||
| serve = serveResult; |
There was a problem hiding this comment.
Instead of storing this in a closure variable, store it at t.context to prevent side effects with other concurrently runnign tests.
See https://github.com/avajs/ava/tree/v0.25.0#test-context
|
|
||
| mock("devcert-sanscache", function(name) { | ||
| t.is(name, "ui5-tooling", "Create certificate for ui5-tooling."); | ||
| return new Promise((resolve) => { |
There was a problem hiding this comment.
Rather use:
return Promise.resolve({
key: sslKey,
cert: sslCert
});
| }); | ||
|
|
||
| mock("devcert-sanscache", function(name) { | ||
| t.is(name, "ui5-tooling", "Create certificate for ui5-tooling."); |
| const sslPathCert = path.join(sslPath, "someOtherServer1.crt"); | ||
| const result = sslUtil.getSslCertificate(sslPathKey, sslPathCert); | ||
| return result.then((res) => { | ||
| t.is(res.key, sslKey, "Key should be returned"); |
| fileExists(sslPathCert) | ||
| ]); | ||
| }).then((fileExistsResult) => { | ||
| t.is(fileExistsResult[0], true, "Key was created."); |
| const sslPathCert = path.join(sslPath, "someOtherServer2.crt"); | ||
| const result = sslUtil.getSslCertificate(sslPathKey, sslPathCert); | ||
| return result.catch((error) => { | ||
| t.is(error, "Certificate installation aborted! Please install the SSL certificate manually.", "Certificate install aborted."); |
There was a problem hiding this comment.
Thanks, I've updated it. Could you please check again.
Updated some test assertions to use t.throws. Removed unnecessary promise rejection handling. Added t.plan to most of the tests.
RandomByte
left a comment
RandomByte
left a comment
There was a problem hiding this comment.
Please squash all commits and use the [INTERNAL] prefix in the squash commit
Added more tests to improve test coverage.
4 participants
Added more tests to improve test coverage.