Skip to content

Commit

Permalink
refactor: change confusing behavior of showing 0 commints when upstre…
Browse files Browse the repository at this point in the history
…am API fails (anuraghazra#3238)
  • Loading branch information
qwerty541 authored and jacobbexten committed Nov 6, 2024
1 parent 43aa4b0 commit f046205
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 60 deletions.
21 changes: 11 additions & 10 deletions src/fetchers/stats-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ const statsFetcher = async (username, ownerAffiliations) => {
*/
const totalCommitsFetcher = async (username) => {
if (!githubUsernameRegex.test(username)) {
logger.log("Invalid username");
return 0;
logger.log("Invalid username provided.");
throw new Error("Invalid username provided.");
}

// https://developer.github.com/v3/search/#search-commits
Expand All @@ -177,18 +177,19 @@ const totalCommitsFetcher = async (username) => {
});
};

let res;
try {
let res = await retryer(fetchTotalCommits, { login: username });
let total_count = res.data.total_count;
if (!!total_count && !isNaN(total_count)) {
return res.data.total_count;
}
res = await retryer(fetchTotalCommits, { login: username });
} catch (err) {
logger.log(err);
throw new Error(err);
}

const totalCount = res.data.total_count;
if (!totalCount || isNaN(totalCount)) {
throw new Error("Could not fetch total commits.");
}
// just return 0 if there is something wrong so that
// we don't break the whole app
return 0;
return totalCount;
};

/**
Expand Down
58 changes: 8 additions & 50 deletions tests/fetchStats.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,62 +210,20 @@ describe("Test fetchStats", () => {
});
});

it("should return 0 commits when all_commits true and invalid username", async () => {
let stats = await fetchStats("asdf///---", true);
expect(stats).toStrictEqual({
contributedTo: 61,
name: "Anurag Hazra",
totalCommits: 0,
totalIssues: 200,
totalPRs: 300,
totalPRsMerged: 240,
mergedPRsPercentage: 80,
totalReviews: 50,
totalStars: 300,
totalDiscussionsStarted: 10,
totalDiscussionsAnswered: 40,
rank: calculateRank({
all_commits: true,
commits: 0,
prs: 300,
reviews: 50,
issues: 200,
repos: 5,
stars: 300,
followers: 100,
}),
});
it("should throw specific error when include_all_commits true and invalid username", async () => {
expect(fetchStats("asdf///---", true)).rejects.toThrow(
new Error("Invalid username provided."),
);
});

it("should return 0 commits when all_commits true and API returns error", async () => {
it("should throw specific error when include_all_commits true and API returns error", async () => {
mock
.onGet("https://api.github.com/search/commits?q=author:anuraghazra")
.reply(200, { error: "Some test error message" });

let stats = await fetchStats("anuraghazra", true);
expect(stats).toStrictEqual({
contributedTo: 61,
name: "Anurag Hazra",
totalCommits: 0,
totalIssues: 200,
totalPRs: 300,
totalPRsMerged: 240,
mergedPRsPercentage: 80,
totalReviews: 50,
totalStars: 300,
totalDiscussionsStarted: 10,
totalDiscussionsAnswered: 40,
rank: calculateRank({
all_commits: true,
commits: 0,
prs: 300,
reviews: 50,
issues: 200,
repos: 5,
stars: 300,
followers: 100,
}),
});
expect(fetchStats("anuraghazra", true)).rejects.toThrow(
new Error("Could not fetch total commits."),
);
});

it("should exclude stars of the `test-repo-1` repository", async () => {
Expand Down

0 comments on commit f046205

Please sign in to comment.