Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@driazati
driazati committed Apr 12, 2022
1 parent 314c42f commit f5c2a19
Show file tree
Hide file tree
Showing 4 changed files with 206 additions and 52 deletions.
123 changes: 123 additions & 0 deletions tests/python/ci/sample_prs/pr10786-missing-job.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
{
"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",
"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/definitely-not-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": true,
"commit": {
"oid": "6f04bcf57d07f915a98fd91178f04d9e92a09fcd"
},
"state": "APPROVED"
}
]
}
}
2 changes: 1 addition & 1 deletion tests/python/ci/sample_prs/pr10786-oldreview.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
"updatedAt": "2022-03-25T22:13:50Z",
"authorCanPushToRepository": true,
"commit": {
"oid": "6f24bcf57d07f915a98fd91178f04d9e92a09fcd"
"oid": "abc12345"
},
"state": "APPROVED"
}
Expand Down
110 changes: 61 additions & 49 deletions tests/python/ci/test_mergebot.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,60 +35,72 @@ def run(self, *args):
raise RuntimeError(f"git command failed: '{args}'")


def test_mergebot(tmpdir_factory):
mergebot_script = REPO_ROOT / "tests" / "scripts" / "github_mergebot.py"
test_json_dir = Path(__file__).resolve().parent / "sample_prs"
test_data = {
"successful-merge": {
"number": 10786,
"filename": "pr10786-merges.json",
"expected": "Dry run, would have merged with url=pulls/10786/merge",
},
"no-request": {
"number": 10786,
"filename": "pr10786-nottriggered.json",
"expected": "No merge requested, exiting",
},
"bad-ci": {
"number": 10786,
"filename": "pr10786-badci.json",
"expected": "Cannot merge, these CI jobs are not successful on",
},
"old-review": {
"number": 10786,
"filename": "pr10786-oldreview.json",
"expected": "Cannot merge, did not find any approving reviews",
},
"missing-job": {
"number": 10786,
"filename": "pr10786-missing-job.json",
"expected": "Cannot merge, missing expected jobs",
},
}

def run(number, filename, expected):
git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
git.run("init")
git.run("checkout", "-b", "main")
git.run("remote", "add", "origin", "https://github.com/apache/tvm.git")
with open(test_json_dir / filename) as f:
test_data = json.load(f)

proc = subprocess.run(
[
str(mergebot_script),
"--pr",
str(number),
"--dry-run",
"--run-url",
"https://example.com",
"--testing-pr-json",
json.dumps(test_data),
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
encoding="utf-8",
cwd=git.cwd,
)
if proc.returncode != 0:
raise RuntimeError(f"Process failed:\nstdout:\n{proc.stdout}\n\nstderr:\n{proc.stderr}")
@pytest.mark.parametrize(
["number", "filename", "expected"],
[tuple(d.values()) for d in test_data.values()],
ids=test_data.keys(),
)
def test_mergebot(tmpdir_factory, number, filename, expected):
mergebot_script = REPO_ROOT / "tests" / "scripts" / "github_mergebot.py"
test_json_dir = Path(__file__).resolve().parent / "sample_prs"

if expected not in proc.stderr:
raise RuntimeError(f"{proc.stderr}\ndid not contain\n{expected}")
git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
git.run("init")
git.run("checkout", "-b", "main")
git.run("remote", "add", "origin", "https://github.com/apache/tvm.git")
with open(test_json_dir / filename) as f:
test_data = json.load(f)

run(
number=10786,
filename="pr10786-merges.json",
expected="Dry run, would have merged with url=pulls/10786/merge",
)
run(
number=10786,
filename="pr10786-nottriggered.json",
expected="No merge requested, exiting",
)
run(
number=10786,
filename="pr10786-badci.json",
expected="Cannot merge, these CI jobs failed on",
)
run(
number=10786,
filename="pr10786-oldreview.json",
expected="Cannot merge, did not find any approving reviews",
proc = subprocess.run(
[
str(mergebot_script),
"--pr",
str(number),
"--dry-run",
"--run-url",
"https://example.com",
"--testing-pr-json",
json.dumps(test_data),
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
encoding="utf-8",
cwd=git.cwd,
)
if proc.returncode != 0:
raise RuntimeError(f"Process failed:\nstdout:\n{proc.stdout}\n\nstderr:\n{proc.stderr}")

if expected not in proc.stderr:
raise RuntimeError(f"{proc.stderr}\ndid not contain\n{expected}")


if __name__ == "__main__":
Expand Down
23 changes: 21 additions & 2 deletions tests/scripts/github_mergebot.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,12 @@ def __init__(
self.repo_name = repo
self.dry_run = dry_run

self.github = GitHubRepo(user=owner, repo=repo, token=os.environ["GITHUB_TOKEN"])

if dry_run and raw_data:
# In test mode there is no need to fetch anything
self.raw = raw_data
self.github = None
else:
self.github = GitHubRepo(user=owner, repo=repo, token=os.environ["GITHUB_TOKEN"])
if os.getenv("DEBUG", "0") == "1":
# For local runs fill in the requested data but cache it for
# later use
Expand Down Expand Up @@ -421,6 +421,25 @@ def merge_if_passed_checks(self):
else:
all_ci_passed = True

# Map of job name: has seen in completed jobs
seen_expected_jobs = {
"tvm-ci/pr-merge": False,
}
logging.info(f"Expected to see jobs: {seen_expected_jobs}")

missing_expected_jobs = []
for job in self.ci_jobs():
seen_expected_jobs[job["name"]] = True

for name, seen in seen_expected_jobs.items():
if not seen:
missing_expected_jobs.append(name)

if len(missing_expected_jobs) > 0:
missing_jobs_msg = "\n".join([f" * `{name}`" for name in missing_expected_jobs])
self.comment(f"Cannot merge, missing expected jobs:\n{missing_jobs_msg}")
return

head_commit_reviews = self.head_commit_reviews()
for review in head_commit_reviews:
if review["state"] == "CHANGES_REQUESTED":
Expand Down

0 comments on commit f5c2a19

Please sign in to comment.