Skip to content

Commit

Permalink
Code to handle draft PRs. Also tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
iarspider committed Dec 6, 2024
1 parent 22973b8 commit 3a04898
Show file tree
Hide file tree
Showing 9 changed files with 536 additions and 14 deletions.
15 changes: 10 additions & 5 deletions process_pr.py
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F

gh_user_char = "@"

if issue.as_pull_request().draft or (not notify_user(issue)):
if not notify_user(issue):
gh_user_char = ""

api_rate_limits(gh)
Expand Down Expand Up @@ -963,12 +963,17 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
warned_too_many_commits = False
ok_too_many_files = False
warned_too_many_files = False
is_draft_pr = False

if issue.pull_request:
pr = repo.get_pull(prId)
if pr.changed_files == 0:
print("Ignoring: PR with no files changed")
return
if pr.draft:
print("Draft PR, mentions turned off")
is_draft_pr = True
gh_user_char = ""
if cmssw_repo and cms_repo and (pr.base.ref == CMSSW_DEVEL_BRANCH):
if pr.state != "closed":
print("This pull request must go in to master branch")
Expand Down Expand Up @@ -1111,7 +1116,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
]
print("PR Statuses:", commit_statuses)
print(len(commit_statuses))
last_commit_date = last_commit.committer.date
last_commit_date = last_commit.committer.date.replace(tzinfo=None)

print(
"Latest commit by ",
Expand Down Expand Up @@ -2488,7 +2493,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
)

messageUpdatedPR = f"Pull request #{pr.number} was updated. "
if not issue.as_pull_request().draft:
if not is_draft_pr:
messageUpdatedPR += (
f"{', '.join(missing_notifications)} can you please check and sign again.\n"
)
Expand Down Expand Up @@ -2519,7 +2524,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
commentMsg = messageBranchClosed
elif (not already_seen) or pull_request_updated:
if not already_seen:
if not issue.as_pull_request().draft:
if not is_draft_pr:
commentMsg = messageNewPR
else:
commentMsg = messageUpdatedPR
Expand Down Expand Up @@ -2555,7 +2560,7 @@ def process_pr(repo_config, gh, repo, issue, dryRun, cmsbuild_user=None, force=F
)
continue
if (not dryRunOrig) and (pre_checks_state[pre_check] == ""):
params = {"PULL_REQUEST": str(prId), "CONTEXT_PREFIX": cms_status_prefix}
params = {"PULL_REQUEST": "%s" % (prId), "CONTEXT_PREFIX": cms_status_prefix}
if pre_check == "code-checks":
params["CMSSW_TOOL_CONF"] = code_checks_tools
params["APPLY_PATCH"] = str(code_check_apply_patch).lower()
Expand Down
57 changes: 57 additions & 0 deletions tests/PRActionData/TestProcessPr.test_draft_pr_assign.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
[
{
"type": "load-bot-cache",
"data": {
"commits": {
"9138474754099798ff50cb07287e7caa4786f247": {
"files": [
"FWCore/Utilities/BuildFile.xml"
],
"squashed": false,
"time": 1733482192
}
},
"emoji": {
"2523442237": "+1"
},
"last_seen_sha": "9138474754099798ff50cb07287e7caa4786f247",
"signatures": {}
}
},
{
"type": "emoji",
"data": [
2523442237,
"+1",
true
]
},
{
"type": "create-comment",
"data": "New categories assigned: l1\n\n@aloeliger,@epalencia you have been requested to review this Pull request/Issue and eventually sign? Thanks"
},
{
"type": "add-label",
"data": [
"l1-pending"
]
},
{
"type": "remove-label",
"data": []
},
{
"type": "edit-comment",
"data": "cms-bot internal usage<!-- bot cache: {\"commits\":{\"9138474754099798ff50cb07287e7caa4786f247\":{\"files\":[\"FWCore/Utilities/BuildFile.xml\"],\"squashed\":false,\"time\":1733482192}},\"emoji\":{\"2523442237\":\"+1\"},\"last_seen_sha\":\"9138474754099798ff50cb07287e7caa4786f247\",\"signatures\":{}} -->"
},
{
"type": "status",
"data": {
"commit": "9138474754099798ff50cb07287e7caa4786f247",
"state": "success",
"target_url": "https://github.com/iarspider-cmssw/cmssw/pull/21#issuecomment-2523442237",
"description": "Comment by iarspider at 2024-12-06 15:01:51 UTC processed.",
"context": "bot/21/ack"
}
}
]
61 changes: 61 additions & 0 deletions tests/PRActionData/TestProcessPr.test_draft_pr_opened.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
[
{
"type": "load-bot-cache",
"data": {
"commits": {
"9138474754099798ff50cb07287e7caa4786f247": {
"files": [
"FWCore/Utilities/BuildFile.xml"
],
"squashed": false,
"time": 1733482192
}
},
"emoji": {},
"last_seen_sha": "9138474754099798ff50cb07287e7caa4786f247",
"signatures": {}
}
},
{
"type": "status",
"data": {
"commit": "9138474754099798ff50cb07287e7caa4786f247",
"state": "pending",
"target_url": null,
"description": "Waiting for authorized user to issue the test command.",
"context": "bot/21/jenkins"
}
},
{
"type": "add-label",
"data": [
"code-checks-approved"
]
},
{
"type": "remove-label",
"data": [
"code-checks-pending"
]
},
{
"type": "status",
"data": {
"commit": "9138474754099798ff50cb07287e7caa4786f247",
"state": "success",
"target_url": "https://github.com/iarspider-cmssw/cmssw/pull/21#issuecomment-2523440118",
"description": "Check details",
"context": "cms/21/code-checks"
}
},
{
"type": "status",
"data": {
"commit": "9138474754099798ff50cb07287e7caa4786f247",
"state": "success",
"target_url": "https://github.com/iarspider-cmssw/cmssw/pull/21#issuecomment-2523440118",
"description": "Comment by iarspider at 2024-12-06 15:00:50 UTC processed.",
"context": "bot/21/ack"
}
}
]
78 changes: 78 additions & 0 deletions tests/PRActionData/TestProcessPr.test_draft_pr_updated.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
[
{
"type": "load-bot-cache",
"data": {
"commits": {
"9138474754099798ff50cb07287e7caa4786f247": {
"files": [
"FWCore/Utilities/BuildFile.xml"
],
"squashed": false,
"time": 1733482192
},
"28ce3f5888b5433dcae409ee336b2ad422595246": {
"time": 1733495641,
"squashed": false,
"files": [
"FWCore/Utilities/BuildFile.xml"
]
}
},
"emoji": {
"2523442237": "+1"
},
"last_seen_sha": "28ce3f5888b5433dcae409ee336b2ad422595246",
"signatures": {}
}
},
{
"type": "emoji",
"data": [
2523442237,
"+1",
true
]
},
{
"type": "status",
"data": {
"commit": "28ce3f5888b5433dcae409ee336b2ad422595246",
"state": "pending",
"target_url": null,
"description": "Waiting for authorized user to issue the test command.",
"context": "bot/21/jenkins"
}
},
{
"type": "add-label",
"data": []
},
{
"type": "remove-label",
"data": []
},
{
"type": "status",
"data": {
"commit": "28ce3f5888b5433dcae409ee336b2ad422595246",
"state": "success",
"target_url": "https://github.com/iarspider-cmssw/cmssw/pull/21#issuecomment-2523514208",
"description": "Check details",
"context": "cms/21/code-checks"
}
},
{
"type": "edit-comment",
"data": "cms-bot internal usage<!-- bot cache: {\"commits\":{\"28ce3f5888b5433dcae409ee336b2ad422595246\":{\"files\":[\"FWCore/Utilities/BuildFile.xml\"],\"squashed\":false,\"time\":1733495641},\"9138474754099798ff50cb07287e7caa4786f247\":{\"files\":[\"FWCore/Utilities/BuildFile.xml\"],\"squashed\":false,\"time\":1733482192}},\"emoji\":{\"2523442237\":\"+1\"},\"last_seen_sha\":\"28ce3f5888b5433dcae409ee336b2ad422595246\",\"signatures\":{}} -->"
},
{
"type": "status",
"data": {
"commit": "28ce3f5888b5433dcae409ee336b2ad422595246",
"state": "success",
"target_url": "https://github.com/iarspider-cmssw/cmssw/pull/21#issuecomment-2523514208",
"description": "Comment by iarspider at 2024-12-06 15:36:43 UTC processed.",
"context": "bot/21/ack"
}
}
]
16 changes: 14 additions & 2 deletions tests/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
* Run `./run_pr_tests.sh [test_name]`, will run all tests if `test_name` is not given

## To record a new test:
* Create `GithubCredentials.py` in top-level directory with contents:

### Setup
* Run tests in replay mode at least once to create venv
* Create `GithubCredentials.py` in top-level directory (**not** in `tests/`) with contents:

```py
login = "<your username>"
Expand All @@ -15,4 +18,13 @@ app_id = ""
app_private_key = ""
```

* Run `./run_pr_tests.sh --record test_name`
* Also write oauth token in `~/.github-token`

### Recording tests

* Prepare (or update) a (cmssw) PR with desired state
* Implement a new test in `test_process_pr.py`. For most tests, only a call to `self.runTest(prId=...)` is needed.
* Run `./process-pull-request.py -n -r <repo> <prId>` to check bot behaviour
* Run `pytest --auth_with_token --record -k test_draft_pr_opened test_process_pr.py` to record PR state and bot actions
* Check recorded actions (`PRActionData/TestProcessPr.<test name>.json`)
* Make bot actually perform actions: `./process-pull-request.py -r <repo> <prId>`
99 changes: 99 additions & 0 deletions tests/ReplayData/TestProcessPr.test_draft_pr_assign.txt

Large diffs are not rendered by default.

99 changes: 99 additions & 0 deletions tests/ReplayData/TestProcessPr.test_draft_pr_opened.txt

Large diffs are not rendered by default.

110 changes: 110 additions & 0 deletions tests/ReplayData/TestProcessPr.test_draft_pr_updated.txt

Large diffs are not rendered by default.

15 changes: 8 additions & 7 deletions tests/test_process_pr.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from .Framework import readLine

actions = []
record_actions = False
record_actions = True


# Utility function for recording calls and optionally calling the original function
Expand Down Expand Up @@ -709,10 +709,11 @@ def test_many_commits_ok(self):
def test_too_many_commits(self):
self.runTest(prId=18)

# Not yet implemented
# def test_future_commit(self):
# self.runTest()
def test_draft_pr_opened(self):
self.runTest(prId=21)

# Not yet implemented
# def test_backdated_commit(self):
# self.runTest()
def test_draft_pr_assign(self):
self.runTest(prId=21)

def test_draft_pr_updated(self):
self.runTest(prId=21)

0 comments on commit 3a04898

Please sign in to comment.