Skip to content

fix: having multiple spec files in a pull request #260

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.

Sign up for GitHub

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

Merged
jalmonter merged 1 commit into from
Nov 25, 2025
Merged

fix: having multiple spec files in a pull request #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 12 additions & 4 deletions src/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,18 @@ export class GitHub {

if (result.status !== 200) {
this.app.log.error('Failed to get main branch files', result);
return undefined;
return;
}

return result.data as { sha: string; name: string; path: string };
if (Array.isArray(result.data)) {
return;
}

if (result.data.type !== 'file') {
return;
}

return result.data;
}

public async listPullRequestFiles({ owner, repo, issueNumber }: CommandContext) {
Expand All @@ -224,7 +232,7 @@ export class GitHub {
return [];
}

return result.data as { filename: string; sha: string }[];
return result.data.map((file) => ({ filename: file.filename, sha: file.sha, status: file.status }));
}

public async fetchFileContents({ owner, repo }: CommandContext, sha: string) {
Expand All @@ -237,7 +245,7 @@ export class GitHub {

if (result.status !== 200) {
this.app.log.error('Failed to get file contents', result);
return '';
return;
}

return Buffer.from(result.data.content, 'base64').toString('utf8');
Expand Down
38 changes: 32 additions & 6 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,15 @@ export default {
const tsSpecFilePath = ensurePath(basePath, 'test/index.spec.ts');
const jsSpecFilePath = ensurePath(basePath, 'test/index.spec.js');

const tsSpecFile = changedFiles.find((file) => file.filename === tsSpecFilePath);
const jsSpecFile = changedFiles.find((file) => file.filename === jsSpecFilePath);
const isValidSpecFile = (file: (typeof changedFiles)[number]) =>
file.status !== 'removed' && file.status !== 'renamed';

const tsSpecFile = changedFiles.find(
(file) => file.filename === tsSpecFilePath && isValidSpecFile(file),
);
const jsSpecFile = changedFiles.find(
(file) => file.filename === jsSpecFilePath && isValidSpecFile(file),
);

// Determine language and file based on what was found
let specFile;
Expand All @@ -242,6 +249,11 @@ export default {
}

const specFileContent = await github.fetchFileContents(context, specFile.sha);
if (!specFileContent) {
const body = `Unable to fetch the contents of the spec file. Please try again.`;
await github.postComment(context, body, workingCommentId);
return;
}

// Analyze the spec file to check for conflicts with Best Practices
const analyzeSpecFilePrompts = buildPromptForAnalyzeSpecFile(specFileContent);
Expand Down Expand Up @@ -331,11 +343,19 @@ export default {
const pullRequestFiles = await github.listPullRequestFiles(context);

// Look for both TS and JS spec files
// Filter out deleted/renamed files to handle partial renames between TS and JS
const tsSpecFilePath = ensurePath(basePath, 'test/index.spec.ts');
const jsSpecFilePath = ensurePath(basePath, 'test/index.spec.js');

const tsSpecFile = pullRequestFiles.find((file) => file.filename === tsSpecFilePath);
const jsSpecFile = pullRequestFiles.find((file) => file.filename === jsSpecFilePath);
const isValidSpecFile = (file: (typeof pullRequestFiles)[number]) =>
file.status !== 'removed' && file.status !== 'renamed';

const tsSpecFile = pullRequestFiles.find(
(file) => file.filename === tsSpecFilePath && isValidSpecFile(file),
);
const jsSpecFile = pullRequestFiles.find(
(file) => file.filename === jsSpecFilePath && isValidSpecFile(file),
);

// Determine language and file based on what was found
let specFile;
Expand Down Expand Up @@ -391,6 +411,7 @@ export default {
indexFile = {
...existingIndexFile,
filename: existingIndexFile.name,
status: 'unchanged'
};

// Determine language based on the file extension if we found a file
Expand All @@ -402,8 +423,13 @@ export default {
}

// Fetch the contents of the spec file and index file
const specFileContent = await github.fetchFileContents(context, specFile.sha);
const indexFileContent = await github.fetchFileContents(context, indexFile!.sha);
const specFileContent = await github.fetchFileContents(context, specFile.sha) ?? '';
const indexFileContent = await github.fetchFileContents(context, indexFile.sha);
if (!indexFileContent) {
const body = `Unable to fetch the contents of the index file. Please try again.`;
await github.postComment(context, body, workingCommentId);
return;
}

const reviewerFeedback = command.extra;
if (!reviewerFeedback) {
Expand Down