Skip to content

Commit 4b8b843

Browse files
committed
[ci] Enable pylint for tests/python/ci
This fixes up the pylint issues as part of #11414 for the CI tests
1 parent e7f793d commit 4b8b843

File tree

6 files changed

+140
-74
lines changed

6 files changed

+140
-74
lines changed

tests/lint/pylint.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,5 @@ python3 -m pylint python/tvm --rcfile="$(dirname "$0")"/pylintrc
2121
python3 -m pylint vta/python/vta --rcfile="$(dirname "$0")"/pylintrc
2222
python3 -m pylint tests/python/unittest/test_tvmscript_type.py --rcfile="$(dirname "$0")"/pylintrc
2323
python3 -m pylint tests/python/contrib/test_cmsisnn --rcfile="$(dirname "$0")"/pylintrc
24+
python3 -m pylint tests/python/ci --rcfile="$(dirname "$0")"/pylintrc
2425

tests/python/ci/__init__.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
"""Infrastructure and tests for CI scripts"""

tests/python/ci/test_ci.py

Lines changed: 66 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -14,32 +14,21 @@
1414
# KIND, either express or implied. See the License for the
1515
# specific language governing permissions and limitations
1616
# under the License.
17-
17+
"""Test various CI scripts and GitHub Actions workflows"""
1818
import subprocess
19-
import sys
2019
import json
21-
from tempfile import tempdir
2220
import textwrap
23-
import pytest
24-
import tvm.testing
2521
from pathlib import Path
2622

27-
from test_utils import REPO_ROOT
28-
29-
30-
class TempGit:
31-
def __init__(self, cwd):
32-
self.cwd = cwd
33-
34-
def run(self, *args, **kwargs):
35-
proc = subprocess.run(["git"] + list(args), encoding="utf-8", cwd=self.cwd, **kwargs)
36-
if proc.returncode != 0:
37-
raise RuntimeError(f"git command failed: '{args}'")
38-
39-
return proc
23+
import pytest
24+
import tvm.testing
25+
from .test_utils import REPO_ROOT, TempGit
4026

4127

4228
def test_cc_reviewers(tmpdir_factory):
29+
"""
30+
Test that reviewers are added from 'cc @someone' messages in PRs
31+
"""
4332
reviewers_script = REPO_ROOT / "tests" / "scripts" / "github_cc_reviewers.py"
4433

4534
def run(pr_body, requested_reviewers, existing_review_users, expected_reviewers):
@@ -49,7 +38,7 @@ def run(pr_body, requested_reviewers, existing_review_users, expected_reviewers)
4938
git.run("remote", "add", "origin", "https://github.com/apache/tvm.git")
5039
reviews = [{"user": {"login": r}} for r in existing_review_users]
5140
requested_reviewers = [{"login": r} for r in requested_reviewers]
52-
proc = subprocess.run(
41+
proc = subprocess.run( # pylint: disable=subprocess-run-check
5342
[str(reviewers_script), "--dry-run", "--testing-reviews-json", json.dumps(reviews)],
5443
stdout=subprocess.PIPE,
5544
stderr=subprocess.PIPE,
@@ -113,6 +102,9 @@ def run(pr_body, requested_reviewers, existing_review_users, expected_reviewers)
113102

114103

115104
def test_update_branch(tmpdir_factory):
105+
"""
106+
Test that the last-successful branch script updates successfully
107+
"""
116108
update_script = REPO_ROOT / "tests" / "scripts" / "update_branch.py"
117109

118110
def run(statuses, expected_rc, expected_output):
@@ -132,7 +124,7 @@ def run(statuses, expected_rc, expected_output):
132124
}
133125
}
134126
}
135-
proc = subprocess.run(
127+
proc = subprocess.run( # pylint: disable=subprocess-run-check
136128
[str(update_script), "--dry-run", "--testonly-json", json.dumps(data)],
137129
stdout=subprocess.PIPE,
138130
stderr=subprocess.PIPE,
@@ -214,6 +206,9 @@ def run(statuses, expected_rc, expected_output):
214206

215207

216208
def test_skip_ci(tmpdir_factory):
209+
"""
210+
Test that CI is skipped when it should be
211+
"""
217212
skip_ci_script = REPO_ROOT / "tests" / "scripts" / "git_skip_ci.py"
218213

219214
def test(commands, should_skip, pr_title, why):
@@ -228,7 +223,7 @@ def test(commands, should_skip, pr_title, why):
228223
for command in commands:
229224
git.run(*command)
230225
pr_number = "1234"
231-
proc = subprocess.run(
226+
proc = subprocess.run( # pylint: disable=subprocess-run-check
232227
[str(skip_ci_script), "--pr", pr_number, "--pr-title", pr_title], cwd=git.cwd
233228
)
234229
expected = 0 if should_skip else 1
@@ -267,7 +262,8 @@ def test(commands, should_skip, pr_title, why):
267262
],
268263
should_skip=False,
269264
pr_title="[no skip ci] test",
270-
why="ci should not be skipped on a branch with [skip ci] in the last commit but not the PR title",
265+
why="ci should not be skipped on a branch with "
266+
"[skip ci] in the last commit but not the PR title",
271267
)
272268

273269
test(
@@ -307,6 +303,9 @@ def test(commands, should_skip, pr_title, why):
307303

308304

309305
def test_skip_globs(tmpdir_factory):
306+
"""
307+
Test that CI is skipped if only certain files are edited
308+
"""
310309
script = REPO_ROOT / "tests" / "scripts" / "git_skip_ci_globs.py"
311310

312311
def run(files, should_skip):
@@ -316,7 +315,7 @@ def run(files, should_skip):
316315
git.run("checkout", "-b", "main")
317316
git.run("remote", "add", "origin", "https://github.com/apache/tvm.git")
318317

319-
proc = subprocess.run(
318+
proc = subprocess.run( # pylint: disable=subprocess-run-check
320319
[
321320
str(script),
322321
"--files",
@@ -342,9 +341,12 @@ def run(files, should_skip):
342341

343342

344343
def test_ping_reviewers(tmpdir_factory):
344+
"""
345+
Test that reviewers are messaged after a time period of inactivity
346+
"""
345347
reviewers_script = REPO_ROOT / "tests" / "scripts" / "ping_reviewers.py"
346348

347-
def run(pr, check):
349+
def run(pr, check): # pylint: disable=invalid-name
348350
git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
349351
# Jenkins git is too old and doesn't have 'git init --initial-branch'
350352
git.run("init")
@@ -361,7 +363,7 @@ def run(pr, check):
361363
}
362364
}
363365
}
364-
proc = subprocess.run(
366+
proc = subprocess.run( # pylint: disable=subprocess-run-check
365367
[
366368
str(reviewers_script),
367369
"--dry-run",
@@ -486,14 +488,20 @@ def all_time_keys(time):
486488

487489

488490
def assert_in(needle: str, haystack: str):
491+
"""
492+
Check that 'needle' is in 'haystack'
493+
"""
489494
if needle not in haystack:
490495
raise AssertionError(f"item not found:\n{needle}\nin:\n{haystack}")
491496

492497

493498
def test_github_tag_teams(tmpdir_factory):
499+
"""
500+
Check that individuals are tagged from team headers
501+
"""
494502
tag_script = REPO_ROOT / "tests" / "scripts" / "github_tag_teams.py"
495503

496-
def run(type, data, check):
504+
def run(source_type, data, check):
497505
git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
498506
git.run("init")
499507
git.run("checkout", "-b", "main")
@@ -528,9 +536,9 @@ def run(type, data, check):
528536
}
529537
}
530538
env = {
531-
type: json.dumps(data),
539+
source_type: json.dumps(data),
532540
}
533-
proc = subprocess.run(
541+
proc = subprocess.run( # pylint: disable=subprocess-run-check
534542
[
535543
str(tag_script),
536544
"--dry-run",
@@ -549,8 +557,8 @@ def run(type, data, check):
549557
assert_in(check, proc.stdout)
550558

551559
run(
552-
"ISSUE",
553-
{
560+
source_type="ISSUE",
561+
data={
554562
"title": "A title",
555563
"number": 1234,
556564
"user": {
@@ -563,12 +571,12 @@ def run(type, data, check):
563571
""".strip()
564572
),
565573
},
566-
"No one to cc, exiting",
574+
check="No one to cc, exiting",
567575
)
568576

569577
run(
570-
"ISSUE",
571-
{
578+
source_type="ISSUE",
579+
data={
572580
"title": "A title",
573581
"number": 1234,
574582
"user": {
@@ -583,11 +591,11 @@ def run(type, data, check):
583591
""".strip()
584592
),
585593
},
586-
"No one to cc, exiting",
594+
check="No one to cc, exiting",
587595
)
588596

589597
run(
590-
type="ISSUE",
598+
source_type="ISSUE",
591599
data={
592600
"title": "A title",
593601
"number": 1234,
@@ -602,11 +610,12 @@ def run(type, data, check):
602610
something"""
603611
),
604612
},
605-
check="would have updated issues/1234 with {'body': '\\nhello\\n\\nsomething\\n\\ncc @person1 @person2 @person4'}",
613+
check="would have updated issues/1234 with {'body': "
614+
"'\\nhello\\n\\nsomething\\n\\ncc @person1 @person2 @person4'}",
606615
)
607616

608617
run(
609-
type="ISSUE",
618+
source_type="ISSUE",
610619
data={
611620
"title": "A title",
612621
"number": 1234,
@@ -625,7 +634,7 @@ def run(type, data, check):
625634
)
626635

627636
run(
628-
type="ISSUE",
637+
source_type="ISSUE",
629638
data={
630639
"title": "[something] A title",
631640
"number": 1234,
@@ -640,11 +649,12 @@ def run(type, data, check):
640649
something"""
641650
),
642651
},
643-
check="would have updated issues/1234 with {'body': '\\nhello\\n\\nsomething\\n\\ncc @person1 @person2 @person4'}",
652+
check="would have updated issues/1234 with {'body': "
653+
"'\\nhello\\n\\nsomething\\n\\ncc @person1 @person2 @person4'}",
644654
)
645655

646656
run(
647-
type="ISSUE",
657+
source_type="ISSUE",
648658
data={
649659
"title": "[something] A title",
650660
"number": 1234,
@@ -663,7 +673,7 @@ def run(type, data, check):
663673
)
664674

665675
run(
666-
type="PR",
676+
source_type="PR",
667677
data={
668678
"title": "[something] A title",
669679
"number": 1234,
@@ -683,7 +693,7 @@ def run(type, data, check):
683693
)
684694

685695
run(
686-
type="PR",
696+
source_type="PR",
687697
data={
688698
"title": "[something] A title",
689699
"number": 1234,
@@ -703,27 +713,31 @@ def run(type, data, check):
703713
)
704714

705715
run(
706-
type="ISSUE",
716+
source_type="ISSUE",
707717
data={
708718
"title": "[something] A title",
709719
"number": 1234,
710720
"user": {
711721
"login": "person5",
712722
},
713723
"labels": [{"name": "something2"}],
724+
# pylint: disable=line-too-long
714725
"body": textwrap.dedent(
715726
"""
716727
`mold` and `lld` can be a much faster alternative to `ld` from gcc. We should modify our CMakeLists.txt to detect and use these when possible. cc @person1
717728
718729
cc @person4
719730
"""
720731
),
732+
# pylint: enable=line-too-long
721733
},
722-
check="would have updated issues/1234 with {'body': '\\n`mold` and `lld` can be a much faster alternative to `ld` from gcc. We should modify our CMakeLists.txt to detect and use these when possible. cc @person1\\n\\ncc @person2 @person4\\n'}",
734+
check="would have updated issues/1234 with {'body': '\\n`mold` and `lld` can be a much"
735+
" faster alternative to `ld` from gcc. We should modify our CMakeLists.txt to "
736+
"detect and use these when possible. cc @person1\\n\\ncc @person2 @person4\\n'}",
723737
)
724738

725739
run(
726-
type="ISSUE",
740+
source_type="ISSUE",
727741
data={
728742
"title": "[something3] A title",
729743
"number": 1234,
@@ -733,11 +747,12 @@ def run(type, data, check):
733747
"labels": [{"name": "something2"}],
734748
"body": "@person2 @SOME1-ONE-",
735749
},
736-
check="Dry run, would have updated issues/1234 with {'body': '@person2 @SOME1-ONE-\\n\\ncc @person1'}",
750+
check="Dry run, would have updated issues/1234 with"
751+
" {'body': '@person2 @SOME1-ONE-\\n\\ncc @person1'}",
737752
)
738753

739754
run(
740-
type="ISSUE",
755+
source_type="ISSUE",
741756
data={
742757
"title": "[] A title",
743758
"number": 1234,
@@ -781,6 +796,9 @@ def run(type, data, check):
781796
],
782797
)
783798
def test_should_rebuild_docker(tmpdir_factory, changed_files, name, check, expected_code):
799+
"""
800+
Check that the Docker images are built when necessary
801+
"""
784802
tag_script = REPO_ROOT / "tests" / "scripts" / "should_rebuild_docker.py"
785803

786804
git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
@@ -824,7 +842,7 @@ def test_should_rebuild_docker(tmpdir_factory, changed_files, name, check, expec
824842
},
825843
}
826844

827-
proc = subprocess.run(
845+
proc = subprocess.run( # pylint: disable=subprocess-run-check
828846
[
829847
str(tag_script),
830848
"--testing-docker-data",

0 commit comments

Comments
 (0)