chore: add tests #9
Conversation
WalkthroughThe codebase has been updated to enhance the functionality related to contributors. New export functions for fetching all contributors and repository-specific contributors have been added. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 5
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- package-lock.json
- package.json
Files selected for processing (4)
- contributors.js (3 hunks)
- test/contributors.test.js (1 hunks)
- test/fixtures/contributors.fixture.js (1 hunks)
- test/index.test.js (1 hunks)
Additional comments: 5
contributors.js (2)
6-11: The addition of new exports
getAllContributorsandgetRepoContributorsand the import of thepathmodule are consistent with the PR's objective to enhance the codebase's functionality. Ensure that the new functions are covered by tests since this PR is focused on adding tests.79-82: The update to the JSDoc comment for
getRepoContributorsto include thepageNoparameter is good for documentation clarity. Verify that the implementation of pagination is correct and that thepageNoparameter is being used as intended in the function.test/contributors.test.js (2)
10-19: The test case for
getRepoContributorsis skipped. Ensure there is a tracking mechanism (like a TODO or an issue) to enable this test once the related code is stable or the blocking issue is resolved.21-30: Similarly, the test case for
getAllContributorsis also skipped. It's important to document the reason for skipping and have a plan for when these tests will be revisited.test/fixtures/contributors.fixture.js (1)
- 1-70: The new test fixture data looks well-structured and appropriate for testing purposes. However, please confirm that the data, especially URLs and node IDs, does not contain sensitive information. Additionally, consider if the
repofield in theVALID_CONTRIBUTOR_SAMPLEobject is necessary given the more detailedreposarray. Lastly, evaluate if theallContributedReposfield would be more useful as an array or object rather than a concatenated string.
|
|
||
| describe.skip('#getRepoContributors('+contributorsFixture.VALID_REPO+');', async function() { | ||
| it('should return the repo contribuors', async function() { | ||
| this.timeout(100000); |
There was a problem hiding this comment.
The timeout for these tests is set to 100000 milliseconds, which is quite long. If these tests are making real HTTP requests, consider using mocks or stubs to simulate API responses for faster and more reliable tests.
Also applies to: 23-23
test/index.test.js
Outdated
| let contributorsHandlesString = await OCK.contributors.github.archive(contributorsFixture.VALID_REPO_OWNER); | ||
| assert.isNotNull(contributorsHandlesString); | ||
| expect(contributorsHandlesString).to.be.not.empty; | ||
| expect(str).to.include('@'); |
There was a problem hiding this comment.
The variable str is not defined in this scope. It seems like it should be contributorsHandlesString.
- expect(str).to.include('@');
+ expect(contributorsHandlesString).to.include('@');Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| expect(str).to.include('@'); | |
| expect(contributorsHandlesString).to.include('@'); |
There was a problem hiding this comment.
@gitcommitshow, thank you for the feedback! I'm glad the suggestion was helpful. If there's anything else I can assist with, just let me know! 🐰
If you found the review helpful, we would love if you give us a shout-out on X (twitter).
contributors.js
Outdated
| * @param {Array} contributors List of contributors details with their contributions metrics | ||
| * @param {Object} options | ||
| * @param {string} options.archiveFolder where to save the final content | ||
| * @param {string} options.archiveFileName the name of the archive file, the content will be overwritten if it exists already | ||
| */ | ||
| function writeContributorLeaderboardToFile(contributors) { | ||
| function writeContributorLeaderboardToFile(contributors, options={}) { | ||
| if(!contributors || contributors.length<1){ | ||
| return; | ||
| } | ||
| const ARCHIVE_FOLDER = options.archiveFolder || process.cwd(); | ||
| const ARCHIVE_FULL_PATH = path.join(ARCHIVE_FOLDER, options.archiveFileName || 'archive-gh-contributors-leaderboard.csv'); | ||
| const fs = require('fs'); | ||
| let ghContributorLeaderboard = contributors.map((contributor) => { | ||
| return ["@" + contributor.login, contributor.contributions, contributor.html_url, contributor.avatar_url, contributor.topContributedRepo, contributor.allContributedRepos].join(); | ||
| }).join("\n"); | ||
| ghContributorLeaderboard = "Github Username,Total Contributions,Profile,Avatar,Most Contribution To,Contributed To\n" + ghContributorLeaderboard; | ||
| fs.writeFile("./gh-contributors-leaderboard.csv", ghContributorLeaderboard, { flag: 'a+' }, function (err) { | ||
| fs.writeFile(ARCHIVE_FULL_PATH, ghContributorLeaderboard, { flag: 'a+' }, function (err) { |
There was a problem hiding this comment.
The modification of writeContributorLeaderboardToFile to accept an options object is a good practice for function extensibility. However, the fs.writeFile method is being used with the { flag: 'a+' } option, which appends to the file if it exists or creates a new file if it does not. This could lead to unexpected behavior if the file already exists, as it would append the new content to the old content. Consider if overwriting the file is the desired behavior and use { flag: 'w' } if so.
- fs.writeFile(ARCHIVE_FULL_PATH, ghContributorLeaderboard, { flag: 'a+' }, function (err) {
+ fs.writeFile(ARCHIVE_FULL_PATH, ghContributorLeaderboard, { flag: 'w' }, function (err) {Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| * @param {Array} contributors List of contributors details with their contributions metrics | |
| * @param {Object} options | |
| * @param {string} options.archiveFolder where to save the final content | |
| * @param {string} options.archiveFileName the name of the archive file, the content will be overwritten if it exists already | |
| */ | |
| function writeContributorLeaderboardToFile(contributors) { | |
| function writeContributorLeaderboardToFile(contributors, options={}) { | |
| if(!contributors || contributors.length<1){ | |
| return; | |
| } | |
| const ARCHIVE_FOLDER = options.archiveFolder || process.cwd(); | |
| const ARCHIVE_FULL_PATH = path.join(ARCHIVE_FOLDER, options.archiveFileName || 'archive-gh-contributors-leaderboard.csv'); | |
| const fs = require('fs'); | |
| let ghContributorLeaderboard = contributors.map((contributor) => { | |
| return ["@" + contributor.login, contributor.contributions, contributor.html_url, contributor.avatar_url, contributor.topContributedRepo, contributor.allContributedRepos].join(); | |
| }).join("\n"); | |
| ghContributorLeaderboard = "Github Username,Total Contributions,Profile,Avatar,Most Contribution To,Contributed To\n" + ghContributorLeaderboard; | |
| fs.writeFile("./gh-contributors-leaderboard.csv", ghContributorLeaderboard, { flag: 'a+' }, function (err) { | |
| fs.writeFile(ARCHIVE_FULL_PATH, ghContributorLeaderboard, { flag: 'a+' }, function (err) { | |
| fs.writeFile(ARCHIVE_FULL_PATH, ghContributorLeaderboard, { flag: 'w' }, function (err) { |
| assert.isNotNull(contributorsHandlesString); | ||
| expect(contributorsHandlesString).to.be.not.empty; | ||
| expect(str).to.include('@'); | ||
| // expect(str).to.include(','); |
There was a problem hiding this comment.
There is a commented-out assertion. If it's not needed, it should be removed; otherwise, add a comment explaining why it's commented out.
contributors.js
Outdated
| /** | ||
| * Writes all contributors data to a file | ||
| * @param {Array} contributors | ||
| * @param {Array} contributors List of contributors details with their contributions metrics | ||
| * @param {Object} options | ||
| * @param {string} options.archiveFolder where to save the final content | ||
| * @param {string} options.archiveFileName the name of the archive file, the content will be overwritten if it exists already | ||
| */ | ||
| function writeContributorLeaderboardToFile(contributors) { | ||
| function writeContributorLeaderboardToFile(contributors, options={}) { | ||
| if(!contributors || contributors.length<1){ | ||
| return; | ||
| } | ||
| const ARCHIVE_FOLDER = options.archiveFolder || process.cwd(); | ||
| const ARCHIVE_FULL_PATH = path.join(ARCHIVE_FOLDER, options.archiveFileName || 'archive-gh-contributors-leaderboard.csv'); | ||
| const fs = require('fs'); | ||
| let ghContributorLeaderboard = contributors.map((contributor) => { | ||
| return ["@" + contributor.login, contributor.contributions, contributor.html_url, contributor.avatar_url, contributor.topContributedRepo, contributor.allContributedRepos].join(); | ||
| }).join("\n"); | ||
| ghContributorLeaderboard = "Github Username,Total Contributions,Profile,Avatar,Most Contribution To,Contributed To\n" + ghContributorLeaderboard; | ||
| fs.writeFile("./gh-contributors-leaderboard.csv", ghContributorLeaderboard, { flag: 'a+' }, function (err) { | ||
| fs.writeFile(ARCHIVE_FULL_PATH, ghContributorLeaderboard, { flag: 'a+' }, function (err) { | ||
| if (err) { | ||
| return console.log(err); | ||
| } |
There was a problem hiding this comment.
The archiveContributorsLeaderboard function calls writeContributorLeaderboardToFile without passing the options object. This could lead to the function not using the intended archive path and file name. Ensure that the options object is passed correctly to writeContributorLeaderboardToFile to reflect the changes made to the function signature.
- writeContributorLeaderboardToFile(contributors);
+ writeContributorLeaderboardToFile(contributors, options);Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| /** | |
| * Writes all contributors data to a file | |
| * @param {Array} contributors | |
| * @param {Array} contributors List of contributors details with their contributions metrics | |
| * @param {Object} options | |
| * @param {string} options.archiveFolder where to save the final content | |
| * @param {string} options.archiveFileName the name of the archive file, the content will be overwritten if it exists already | |
| */ | |
| function writeContributorLeaderboardToFile(contributors) { | |
| function writeContributorLeaderboardToFile(contributors, options={}) { | |
| if(!contributors || contributors.length<1){ | |
| return; | |
| } | |
| const ARCHIVE_FOLDER = options.archiveFolder || process.cwd(); | |
| const ARCHIVE_FULL_PATH = path.join(ARCHIVE_FOLDER, options.archiveFileName || 'archive-gh-contributors-leaderboard.csv'); | |
| const fs = require('fs'); | |
| let ghContributorLeaderboard = contributors.map((contributor) => { | |
| return ["@" + contributor.login, contributor.contributions, contributor.html_url, contributor.avatar_url, contributor.topContributedRepo, contributor.allContributedRepos].join(); | |
| }).join("\n"); | |
| ghContributorLeaderboard = "Github Username,Total Contributions,Profile,Avatar,Most Contribution To,Contributed To\n" + ghContributorLeaderboard; | |
| fs.writeFile("./gh-contributors-leaderboard.csv", ghContributorLeaderboard, { flag: 'a+' }, function (err) { | |
| fs.writeFile(ARCHIVE_FULL_PATH, ghContributorLeaderboard, { flag: 'a+' }, function (err) { | |
| if (err) { | |
| return console.log(err); | |
| } | |
| // No changes required in the provided code snippet |
add tests using mocha and chai
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation