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
Add an option to use github's auto merge #302
Conversation
95016ad
to
f337f3e
Compare
src/github-client.js
Outdated
async enableAutoMergePullRequest(pullRequestId, mergeMethod) { | ||
const query = ` | ||
mutation ($pullRequestId: ID!, $mergeMethod: PullRequestMergeMethod!) { | ||
enablePullRequestAutoMerge(input: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did you tested it?
I wonder if the GHUser that runs this GHA has the permission to execute this mutation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried in a private repo of the company I work for but I'm gonna create a public repo and paste the url soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can see it in action shouichi#1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if the mutation fails (ex: user does not have the permission )? Can you add a test for the failed mutation?
Do you mean a uni test or an integration test? |
62bad73
to
e8dfb66
Compare
the ideal would be to test it in a real case scenario and add a unit test to simulate the github error (I'm not 100% what happens if the Github user doesn't have permission or if it is required) |
But existing tests only cover happy paths and github-actoions.js doesn't even check the return value from the api... |
Ok here's my logical answer. That kind of test won't really add value because it only tests GitHub behavior, no the script. The behavior of the script is already covered and that should be enough. What do you think? |
I've tested it on my repo, LGTM |
Thank you! |
I think |
It does not skip status checks, it only merges when all required status checks were successful. How about |
👍 |
e8dfb66
to
031f9f7
Compare
Fixed. Could you take a look? |
031f9f7
to
882c946
Compare
Sorry, forgot to rename the option name in the test. Fixed and confirmed tests were passed. |
1080a70
to
85de44b
Compare
85de44b
to
ead0794
Compare
Thank you! |
Close #301.