[INTERNAL] improve test coverage#120
Conversation
|
|
tobiasso85
changed the title
|
Apparently, you merged the master into this branch. This should have rather been a rebase. We should remove the merge commit and do a rebase 👍 |
tobiasso85
force-pushed
the
terst-coverage-abstract-formatter
branch
from
39310bc to
a82118e
Compare
RandomByte
left a comment
RandomByte
left a comment
There was a problem hiding this comment.
Roman was right, you are actually adding 3k lines of test code 👍
Maybe @codeworrior should also have a look as a lot of lbt-modules are being tested here.
For many assertions there is no message given. These should be fairly easy to add while making it significantly easier to debug failing tests.
I only got until `test/lib/tasks/bundlers/generateLibraryPreload.js´. Will do the rest in a separate review.
| * @private | ||
| */ | ||
| function createDependencyGraph(pool, moduleNames, indegreeOnly) { | ||
| async function createDependencyGraph(pool, moduleNames, indegreeOnly) { |
There was a problem hiding this comment.
Whats the reason for this change? The function already returns a promise
There was a problem hiding this comment.
just wanted to mark it accordingly, but I leave it out and adjust the jsdoc accordingly
| * @private | ||
| */ | ||
| function topologicalSort(pool, moduleNames) { | ||
| async function topologicalSort(pool, moduleNames) { |
| log.verbose(" scanning <%s> for resources (prefix='%s')", fileOrFolder, prefix); | ||
| const p = scanFileOrDir(fileOrFolder, prefix, this); | ||
| this.whenReady = this.whenReady.then( () => p ); | ||
| return this.whenReady; |
There was a problem hiding this comment.
Whats the reason for this change? This function used to be synchronous. I wondered why nothing breaks but it seems that this function isn't used anywhere 😕
Still, even when keeping this code, I think this change is not necessary in order to test it.
There was a problem hiding this comment.
But then you never know when the function succeeded, because scanFileOrDir is async otherwise I wasn't able to test it and scanFileOrDir is private (cannot be mocked)
There was a problem hiding this comment.
put this.whenReady resolves only after scanFilOrDir has resolved. And that's what you wanted isn't it?
There was a problem hiding this comment.
exactly, otherwise I wasn't able to determine when the method finished
| * Checks whether or not the given input is a directory on the file system. | ||
| * @param {string} dirPath directory | ||
| * @returns {Promise<boolean>} whether or not the given directory exists. | ||
| * <code>true</code> if file exists is a directory |
There was a problem hiding this comment.
if file exists is a directory
What about "Directory exists"?
There was a problem hiding this comment.
I changed it to make it a bit more readable
test coverage for AbstractBuilder Fixes "Unknown" typo Fixes missing arguments
Adds documentation to addType and getType
Adds sinon as dev dependency to be able to stub/mock/spy in tests
adds tests for LibraryFormatter and libraryType
Covers sap/ui/core specifics
comment out ResourcePool#addRoot since it is not used
Fixes AutoSplitter for resource infos which do not have a size. Ensures size is either a positive number or 0
tests individual functions in modules
Adds LibraryFormatter tests for .library files
* replace t.is with t.deepEqual for string comparisons of error message * mock fs for AbstractBuilder test
tobiasso85
force-pushed
the
terst-coverage-abstract-formatter
branch
from
7a87a2f to
c02c1bb
Compare
4 participants
Adds ava test for AbstractFormatter
Add JSDoc for dirExists function