Skip to content

Sonalmahajan15/add grouping argument to golden test#6

Open
lizard-boy wants to merge 5 commits into
mainfrom
sonalmahajan15/add-grouping-argument-to-golden-test
Open

Sonalmahajan15/add grouping argument to golden test#6
lizard-boy wants to merge 5 commits into
mainfrom
sonalmahajan15/add-grouping-argument-to-golden-test

Conversation

@lizard-boy

Copy link
Copy Markdown

No description provided.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

PR Summary

This PR introduces a grouping argument to the golden test functionality, enabling tests to run in both grouped and ungrouped modes.

• tools/cmd/golden-test/main_test.go now tests WriteDiff with an explicit grouping flag ("true").
• .github/workflows/golden-test.yml adds a matrix strategy to toggle grouping; ensure the header for bot updates matches exactly.
• tools/cmd/golden-test/main.go propagates the grouping argument to both the NilAway binary call and the diff header text.
• Consider switching error logging from the standard log package to the custom logger.error format per guidelines.

3 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

issue_number: issueNumber
});
const botComment = comments.data.find(comment => comment.user.login === 'github-actions[bot]' && comment.body.startsWith('## Golden Test'));
const botComment = comments.data.find(comment => comment.user.login === 'github-actions[bot]' && comment.body.startsWith('## Golden Test ${{ matrix.grouping }}'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

logic: Check that the header string produced by WriteDiff (e.g. '## Golden Test w/ grouping' or '## Golden Test w/o grouping') exactly matches the expectation in the github-script's botComment check. Any discrepancy may prevent updating the existing comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants