Skip to content

Commit

Permalink
Address comments, add author filter
Browse files Browse the repository at this point in the history
  • Loading branch information
driazati committed May 11, 2022
1 parent 69e4cbf commit a43de88
Show file tree
Hide file tree
Showing 9 changed files with 215 additions and 14 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/merge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ concurrency:

jobs:
maybe-merge:
if: github.repository == 'driazati/tvm'
# if: github.repository == 'apache/tvm' # TODO: uncomment after testing
if: github.repository == 'apache/tvm'
runs-on: ubuntu-20.04
steps:
- uses: actions/checkout@v2
Expand Down
10 changes: 8 additions & 2 deletions tests/python/ci/sample_prs/pr10786-badci.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"title": "[Hexagon] 2-d allocation cleanup",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"state": "OPEN",
"author": {
"login": "Lunderberg"
},
"comments": {
"pageInfo": {
"hasPreviousPage": false
Expand Down Expand Up @@ -62,7 +65,7 @@
}
},
"status": "COMPLETED",
"conclusion": "FAILED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/5694945029"
},
{
Expand Down Expand Up @@ -92,7 +95,7 @@
"url": "https://github.com/apache/tvm/runs/5694945524"
},
{
"state": "SUCCESS",
"state": "FAILED",
"context": "tvm-ci/pr-merge",
"targetUrl": "https://ci.tlcpack.ai/job/tvm/job/PR-10786/1/display/redirect"
}
Expand All @@ -116,6 +119,9 @@
"commit": {
"oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd"
},
"author": {
"login": "kparzysz-quic"
},
"state": "APPROVED"
}
]
Expand Down
129 changes: 129 additions & 0 deletions tests/python/ci/sample_prs/pr10786-invalid-author.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
{
"title": "[Hexagon] 2-d allocation cleanup",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"state": "OPEN",
"author": {
"login": "Lunderberg"
},
"comments": {
"pageInfo": {
"hasPreviousPage": false
},
"nodes": []
},
"authorCommits": {
"nodes": [
{
"commit": {
"authors": {
"nodes": [
{
"name": "Eric Lunderberg",
"email": "elunderberg@octoml.ai"
},
{
"name": "Adam Straw",
"email": "astraw@octoml.ai"
}
]
}
}
}
]
},
"commits": {
"nodes": [
{
"commit": {
"oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd",
"statusCheckRollup": {
"contexts": {
"pageInfo": {
"hasNextPage": false
},
"nodes": [
{
"name": "MacOS",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "CI"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/5694945392"
},
{
"name": "cc-reviewers",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "PR"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/5694945029"
},
{
"name": "tag-teams",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "Teams"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/5694945030"
},
{
"name": "Windows",
"checkSuite": {
"workflowRun": {
"workflow": {
"name": "CI"
}
}
},
"status": "COMPLETED",
"conclusion": "SUCCESS",
"url": "https://github.com/apache/tvm/runs/5694945524"
},
{
"state": "SUCCESS",
"context": "tvm-ci/pr-merge",
"targetUrl": "https://ci.tlcpack.ai/job/tvm/job/PR-10786/1/display/redirect"
}
]
}
}
}
}
]
},
"reviewDecision": "APPROVED",
"reviews": {
"pageInfo": {
"hasPreviousPage": false
},
"nodes": [
{
"body": "@tvm-bot merge",
"updatedAt": "2022-03-25T22:13:50Z",
"authorCanPushToRepository": false,
"commit": {
"oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd"
},
"author": {
"login": "kparzysz-quic"
},
"state": "APPROVED"
}
]
}
}
6 changes: 6 additions & 0 deletions tests/python/ci/sample_prs/pr10786-merges.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"title": "[Hexagon] 2-d allocation cleanup",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"state": "OPEN",
"author": {
"login": "Lunderberg"
},
"comments": {
"pageInfo": {
"hasPreviousPage": false
Expand Down Expand Up @@ -116,6 +119,9 @@
"commit": {
"oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd"
},
"author": {
"login": "kparzysz-quic"
},
"state": "APPROVED"
}
]
Expand Down
6 changes: 6 additions & 0 deletions tests/python/ci/sample_prs/pr10786-missing-job.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"title": "[Hexagon] 2-d allocation cleanup",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"state": "OPEN",
"author": {
"login": "Lunderberg"
},
"comments": {
"pageInfo": {
"hasPreviousPage": false
Expand Down Expand Up @@ -116,6 +119,9 @@
"commit": {
"oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd"
},
"author": {
"login": "kparzysz-quic"
},
"state": "APPROVED"
}
]
Expand Down
6 changes: 6 additions & 0 deletions tests/python/ci/sample_prs/pr10786-nottriggered.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"title": "[Hexagon] 2-d allocation cleanup",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"state": "OPEN",
"author": {
"login": "Lunderberg"
},
"comments": {
"pageInfo": {
"hasPreviousPage": false
Expand Down Expand Up @@ -116,6 +119,9 @@
"commit": {
"oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd"
},
"author": {
"login": "kparzysz-quic"
},
"state": "APPROVED"
}
]
Expand Down
6 changes: 6 additions & 0 deletions tests/python/ci/sample_prs/pr10786-oldreview.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"title": "[Hexagon] 2-d allocation cleanup",
"body": "- Added device validity check in allocation. HexagonDeviceAPI should only be called for CPU/Hexagon types.\r\n\r\n- Check for \"global.vtcm\" scope instead of \"vtcm\". The ccope of N-d allocations produced by `LowerVtcmAlloc` should be `\"global.vtcm\"`. The previous check allowed unsupported scope such as `\"local.vtcm\"`.\r\n\r\n- Remove `vtcmallocs` entry after calling free. Previously, the vtcm allocation map kept dangling pointers to `HexagonBuffer` objects after they had been freed.\r\n\r\n- Rename N-d alloc and free packed functions. Since most of the similar device functions use snake case, renaming `*.AllocND` to `*.alloc_nd` and `*.FreeND` to `*.free_nd`.\r\n\r\nCo-authored-by: Adam Straw <astraw@octoml.ai>",
"state": "OPEN",
"author": {
"login": "Lunderberg"
},
"comments": {
"pageInfo": {
"hasPreviousPage": false
Expand Down Expand Up @@ -116,6 +119,9 @@
"commit": {
"oid": "abc12345"
},
"author": {
"login": "kparzysz-quic"
},
"state": "APPROVED"
}
]
Expand Down
15 changes: 13 additions & 2 deletions tests/python/ci/test_mergebot.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,36 +40,47 @@ def run(self, *args):
"number": 10786,
"filename": "pr10786-merges.json",
"expected": "Dry run, would have merged with url=pulls/10786/merge",
"detail": "Everything is fine so this PR will merge",
},
"no-request": {
"number": 10786,
"filename": "pr10786-nottriggered.json",
"expected": "No merge requested, exiting",
"detail": "A PR for which the mergebot runs but no merge is requested",
},
"bad-ci": {
"number": 10786,
"filename": "pr10786-badci.json",
"expected": "Cannot merge, these CI jobs are not successful on",
"detail": "A PR which failed CI and cannot merge",
},
"old-review": {
"number": 10786,
"filename": "pr10786-oldreview.json",
"expected": "Cannot merge, did not find any approving reviews",
"detail": "A PR with passing CI and approving reviews on an old commit so it cannot merge",
},
"missing-job": {
"number": 10786,
"filename": "pr10786-missing-job.json",
"expected": "Cannot merge, missing expected jobs",
"detail": "PR missing an expected CI job and cannot merge",
},
"invalid-author": {
"number": 10786,
"filename": "pr10786-invalid-author.json",
"expected": "No merge requested, exiting",
"detail": "Merge requester is not a committer and cannot merge",
},
}


@pytest.mark.parametrize(
["number", "filename", "expected"],
["number", "filename", "expected", "detail"],
[tuple(d.values()) for d in test_data.values()],
ids=test_data.keys(),
)
def test_mergebot(tmpdir_factory, number, filename, expected):
def test_mergebot(tmpdir_factory, number, filename, expected, detail):
mergebot_script = REPO_ROOT / "tests" / "scripts" / "github_mergebot.py"
test_json_dir = Path(__file__).resolve().parent / "sample_prs"

Expand Down
Loading

0 comments on commit a43de88

Please sign in to comment.