Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add tests for dashboard script #3344

Merged
merged 18 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions scripts/dashboard/build-dashboard.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { writeFileSync } = require('fs');
const { writeFile } = require('fs-extra');
const { resolve } = require('path');
const { graphql } = require('@octokit/graphql');
const { Queries } = require('./issue-queries');
Expand Down Expand Up @@ -44,10 +44,10 @@ async function getDiscussions(query, pageSize, endCursor = null) {
return result.search.nodes.concat(await getDiscussions(query, pageSize, result.search.pageInfo.endCursor));
} catch (e) {
console.error(e);

return Promise.reject(e);
}
}

async function getDiscussionByID(isPR, id) {
try {
const result = await graphql(isPR ? Queries.pullRequestById : Queries.issueById, {
Expand All @@ -60,7 +60,6 @@ async function getDiscussionByID(isPR, id) {
return result;
} catch (e) {
console.error(e);

return Promise.reject(e);
}
}
Expand All @@ -69,7 +68,6 @@ async function processHotDiscussions(batch) {
return Promise.all(
batch.map(async (discussion) => {
try {
// eslint-disable-next-line no-underscore-dangle
const isPR = discussion.__typename === 'PullRequest';
if (discussion.comments.pageInfo.hasNextPage) {
const fetchedDiscussion = await getDiscussionByID(isPR, discussion.id);
Expand All @@ -83,9 +81,10 @@ async function processHotDiscussions(batch) {

const finalInteractionsCount = isPR
? interactionsCount +
discussion.reviews.totalCount +
discussion.reviews.nodes.reduce((acc, curr) => acc + curr.comments.totalCount, 0)
discussion.reviews.totalCount +
discussion.reviews.nodes.reduce((acc, curr) => acc + curr.comments.totalCount, 0)
: interactionsCount;

return {
id: discussion.id,
isPR,
Expand All @@ -111,21 +110,20 @@ async function getHotDiscussions(discussions) {

for (let i = 0; i < discussions.length; i += batchSize) {
const batch = discussions.slice(i, i + batchSize);
// eslint-disable-next-line no-await-in-loop
const batchResults = await processHotDiscussions(batch);

// eslint-disable-next-line no-await-in-loop
await pause(1000);

result.push(...batchResults);
}

result.sort((ElemA, ElemB) => ElemB.score - ElemA.score);
const filteredResult = result.filter((issue) => issue.author !== 'asyncapi-bot');
return filteredResult.slice(0, 12);
}
async function writeToFile(content) {
writeFileSync(resolve(__dirname, '..', '..', 'dashboard.json'), JSON.stringify(content, null, ' '));

async function writeToFile(content, writePath) {
await writeFile(writePath, JSON.stringify(content, null, ' '));
}

async function mapGoodFirstIssues(issues) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add error handling to writeToFile function.

The function should handle potential file system errors gracefully.

Add error handling:

 async function writeToFile(content, writePath) {
+  try {
     await writeFile(writePath, JSON.stringify(content, null, '  '));
+  } catch (error) {
+    console.error('Failed to write dashboard data:', {
+      error: error.message,
+      writePath
+    });
+    throw error;
+  }
 }
📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function writeToFile(content, writePath) {
await writeFile(writePath, JSON.stringify(content, null, ' '));
}
async function writeToFile(content, writePath) {
try {
await writeFile(writePath, JSON.stringify(content, null, ' '));
} catch (error) {
console.error('Failed to write dashboard data:', {
error: error.message,
writePath
});
throw error;
}
}

return issues.map((issue) => ({
id: issue.id,
Expand Down Expand Up @@ -153,7 +151,7 @@ function monthsSince(date) {
return Math.floor(months);
}

async function start() {
async function start(writePath) {
try {
const issues = await getDiscussions(Queries.hotDiscussionsIssues, 20);
const PRs = await getDiscussions(Queries.hotDiscussionsPullRequests, 20);
Expand All @@ -163,12 +161,16 @@ async function start() {
getHotDiscussions(discussions),
mapGoodFirstIssues(rawGoodFirstIssues)
]);
writeToFile({ hotDiscussions, goodFirstIssues });
return await writeToFile({ hotDiscussions, goodFirstIssues }, writePath);
} catch (e) {
console.log('There were some issues parsing data from github.');
console.log(e);
}
}
start();

module.exports = { getLabel, monthsSince, mapGoodFirstIssues, getHotDiscussions, getDiscussionByID };
/* istanbul ignore next */
if (require.main === module) {
start(resolve(__dirname, '..', '..', 'dashboard.json'));
}

module.exports = { getLabel, monthsSince, mapGoodFirstIssues, getHotDiscussions, getDiscussionByID, getDiscussions, writeToFile, start, processHotDiscussions };
197 changes: 197 additions & 0 deletions tests/dashboard/build-dashboard.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
const { graphql } = require('@octokit/graphql');
const { promises: fs, mkdirSync, rmSync } = require('fs-extra');
const { resolve } = require('path');
const os = require('os');
const {
getLabel,
monthsSince,
mapGoodFirstIssues,
getHotDiscussions,
getDiscussionByID,
writeToFile,
getDiscussions,
start
} = require('../../scripts/dashboard/build-dashboard');

const {
issues,
mockDiscussion,
discussionWithMoreComments,
fullDiscussionDetails,
mockRateLimitResponse
} = require("../fixtures/dashboardData")

jest.mock('@octokit/graphql');

describe('GitHub Discussions Processing', () => {
let tempDir;
let consoleErrorSpy;
let consoleLogSpy;

beforeAll(() => {
tempDir = resolve(os.tmpdir(), 'test-config');
mkdirSync(tempDir);
});

afterAll(() => {
rmSync(tempDir, { recursive: true, force: true });
});

beforeEach(() => {
jest.clearAllMocks();
consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => { });
consoleLogSpy = jest.spyOn(console, 'log').mockImplementation(() => { });
});

afterEach(() => {
consoleErrorSpy.mockRestore();
consoleLogSpy.mockRestore();
});

it('should fetch additional discussion details when comments have next page', async () => {
graphql.mockResolvedValueOnce(fullDiscussionDetails);

const result = await getHotDiscussions([discussionWithMoreComments]);

expect(graphql).toHaveBeenCalledWith(
expect.any(String),
expect.objectContaining({
id: 'paginated-discussion',
headers: expect.any(Object)
})
);

expect(result[0]).toMatchObject({
id: 'paginated-discussion',
isPR: false,
title: 'Test with Pagination'
});

const firstResult = result[0];
expect(firstResult.score).toBeGreaterThan(0);
});

it('should handle rate limit warnings', async () => {
graphql.mockResolvedValueOnce(mockRateLimitResponse);

await getDiscussions('test-query', 10);

expect(consoleLogSpy).toHaveBeenCalledWith(
'[WARNING] GitHub GraphQL rateLimit',
'cost = 1',
'limit = 5000',
'remaining = 50',
expect.any(String)
);
});

it('should handle pagination', async () => {
const mockFirstResponse = {
search: {
nodes: [mockDiscussion],
pageInfo: { hasNextPage: true, endCursor: 'cursor1' }
},
rateLimit: { remaining: 1000 }
};

const mockSecondResponse = {
search: {
nodes: [{ ...mockDiscussion, id: 'test-id-2' }],
pageInfo: { hasNextPage: false }
},
rateLimit: { remaining: 1000 }
};

graphql
.mockResolvedValueOnce(mockFirstResponse)
.mockResolvedValueOnce(mockSecondResponse);

const result = await getDiscussions('test-query', 10);
expect(result).toHaveLength(2);
});

it('should handle complete failure', async () => {
graphql.mockRejectedValue(new Error('Complete API failure'));

const filePath = resolve(tempDir, 'error-output.json');
await start(filePath);

expect(consoleLogSpy).toHaveBeenCalledWith('There were some issues parsing data from github.');
});

it('should successfully process and write data', async () => {
graphql.mockResolvedValue(mockRateLimitResponse);

const filePath = resolve(tempDir, 'success-output.json');
await start(filePath);

const content = JSON.parse(await fs.readFile(filePath, 'utf-8'));
expect(content).toHaveProperty('hotDiscussions');
expect(content).toHaveProperty('goodFirstIssues');
});

it('should get labels correctly', () => {
const issue = {
labels: { nodes: [{ name: 'area/bug' }, { name: 'good first issue' }] }
};
expect(getLabel(issue, 'area/')).toBe('bug');
expect(getLabel(issue, 'nonexistent/')).toBeUndefined();
});

it('should calculate months since date', () => {
const date = new Date();
date.setMonth(date.getMonth() - 2);
expect(monthsSince(date)).toBe(2);
});

it('should map good first issues', async () => {

const result = await mapGoodFirstIssues(issues);
expect(result[0]).toMatchObject({
id: '1',
area: 'docs'
});
});

it('should handle discussion retrieval', async () => {
graphql.mockResolvedValueOnce({ node: mockDiscussion });
const result = await getDiscussionByID(false, 'test-id');
expect(result.node).toBeDefined();

graphql.mockRejectedValueOnce(new Error('API error'));
await expect(getDiscussionByID(true, 'test-id')).rejects.toThrow();
});

it('should process hot discussions', async () => {
const prDiscussion = {
...mockDiscussion,
__typename: 'PullRequest',
reviews: {
totalCount: 1,
nodes: [{ comments: { totalCount: 1 } }]
}
};

const result = await getHotDiscussions([mockDiscussion, prDiscussion]);
expect(result.length).toBeLessThanOrEqual(12);
});

it('should write to file', async () => {
const filePath = resolve(tempDir, 'test.json');
await writeToFile({ test: true }, filePath);
const content = JSON.parse(await fs.readFile(filePath, 'utf-8'));
expect(content).toEqual({ test: true });
});

it('should handle parsing errors in processHotDiscussions', async () => {
const consoleErrorSpy = jest.spyOn(console, 'error');

await expect(getHotDiscussions([undefined])).rejects.toThrow();

expect(consoleErrorSpy).toHaveBeenCalledWith(
'there was some issues while parsing this item: undefined'
);

consoleErrorSpy.mockRestore();
});
anshgoyalevil marked this conversation as resolved.
Show resolved Hide resolved
});
81 changes: 81 additions & 0 deletions tests/fixtures/dashboardData.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
const mockDiscussion = {
id: 'test-id',
__typename: 'Issue',
title: 'Test',
author: { login: 'author' },
resourcePath: '/path',
repository: { name: 'repo' },
assignees: { totalCount: 0 },
reactions: { totalCount: 5 },
comments: {
totalCount: 2,
nodes: [{ reactions: { totalCount: 1 } }],
pageInfo: { hasNextPage: false }
},
labels: { nodes: [] },
timelineItems: { updatedAt: new Date().toISOString() }
};

const discussionWithMoreComments = {
id: 'paginated-discussion',
__typename: 'Issue',
title: 'Test with Pagination',
author: { login: 'author' },
resourcePath: '/path',
repository: { name: 'repo' },
assignees: { totalCount: 0 },
reactions: { totalCount: 5 },
comments: {
totalCount: 5,
nodes: [{ reactions: { totalCount: 1 } }],
pageInfo: { hasNextPage: true }
},
labels: { nodes: [] },
timelineItems: { updatedAt: new Date().toISOString() }
};

const fullDiscussionDetails = {
node: {
...discussionWithMoreComments,
comments: {
totalCount: 5,
nodes: [
{ reactions: { totalCount: 1 } },
{ reactions: { totalCount: 2 } },
{ reactions: { totalCount: 3 } }
],
pageInfo: { hasNextPage: false }
}
}
};

const mockRateLimitResponse = {
search: {
nodes: [mockDiscussion],
pageInfo: { hasNextPage: false }
},
rateLimit: {
cost: 1,
limit: 5000,
remaining: 50,
resetAt: new Date().toISOString()
}
};

const issues = [{
id: '1',
title: 'Test',
assignees: { totalCount: 1 },
resourcePath: '/path',
repository: { name: 'repo' },
author: { login: 'author' },
labels: { nodes: [{ name: 'area/docs' }] }
}];

module.exports = {
issues,
mockDiscussion,
discussionWithMoreComments,
fullDiscussionDetails,
mockRateLimitResponse
};
Loading