TypeError when GitHub URL is not defined #52 #53
Conversation
|
@zubinbhasin thanks for the PR. |
src/plugins/archive.js
Outdated
| const { fetchGithub } = require('../lib/fetch'); | ||
|
|
||
| const archivePlugin = async (pkg, _, options) => { | ||
| if (!pkg.repository.url) { | ||
| warning(output.get()); | ||
| return createWarning(`No Github url found for the ${pkg.name} module`); |
There was a problem hiding this comment.
| return createWarning(`No Github url found for the ${pkg.name} module`); | |
| return createWarning(`The module "${pkg.name}" does not specify its GitHub repository.`); |
I have added the unit test and made the changes according to the suggestions by @aalykiot . Please have a look now. |
src/plugins/archive.js
Outdated
| 'Checking if github repository is archived' | ||
| ).withPadding(65); | ||
|
|
||
| if (!pkg.repository.url) { |
There was a problem hiding this comment.
If the repository field is also undefined then the if statement will throw a TypeError.
| if (!pkg.repository.url) { | |
| if (!pkg?.repository?.url) { |
test/plugins/archive.test.js
Outdated
|
|
||
| it('should log warning message when module does not specify its GitHub repository', async () => { | ||
| // mocking http request to GitHub | ||
| network.fetchGithub.mockImplementation(() => { |
There was a problem hiding this comment.
You don't have to mock the network request here since you'll never reach the network call anyway.
|
@zubinbhasin I would suggest writing one test with |
Use the Coveralls GitHub action instead of the module to allow coverage runs on pull requests from forks. Fixes: nodeshift#54
@aalykiot I've covered both of these cases, also removed the mock calls and added optional chaining snippet as mentioned above. |
There was a problem hiding this comment.
@zubinbhasin LGTM, can you please rebase from main so we can merge your PR ??
@aalykiot I've done the rebase but coveralls seems to be failing now. |
Don't need to worry about that coveralls issue 👍 |
#52