[GitHub] [tvm] areusch commented on a diff in pull request #12389: [ci] Check for timestamps in Jenkinsfile changes

2022-08-25 Thread GitBox


areusch commented on code in PR #12389:
URL: https://github.com/apache/tvm/pull/12389#discussion_r955436101


##
tests/python/ci/test_ci.py:
##
@@ -1100,5 +1101,72 @@ def test_should_rebuild_docker(tmpdir_factory, 
changed_files, name, check, expec
 assert proc.returncode == expected_code
 
 
+@tvm.testing.skip_if_wheel_test
+def test_jenkinsfile_timestamps(tmpdir_factory):
+"""
+Check that the timestamp added by ci/jenkins/generate.py is correctly
+checked with --check
+"""
+# Git repo set up
+git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
+git.run("init")
+git.run("config", "user.name", "ci")
+git.run("config", "user.email", "em...@example.com")
+git.run("checkout", "-b", "main")
+git.run("remote", "add", "origin", "https://github.com/apache/tvm.git";)
+cwd = Path(git.cwd)
+
+# generate.py relies on files being at certain places in the repo, so copy
+# them all over
+shutil.copytree(REPO_ROOT / "ci", cwd / "ci")
+script = cwd / "ci" / "jenkins" / "generate.py"
+
+# generate the Jenkinsfile based on the copied templates as a starting 
point
+with open(cwd / "Jenkinsfile", "w") as f:
+f.write("abc")
+
+subprocess.run(
+[
+str(script),
+"--base-ref",
+"HEAD~1",
+],
+encoding="utf-8",
+stdout=subprocess.PIPE,
+cwd=git.cwd,
+check=True,
+)
+
+git.run("add", ".")
+git.run("commit", "-mtest")
+
+# Edit the Jenkinsfile and templates without updating the timestamp
+with open(cwd / "Jenkinsfile", "a") as f:
+f.write("test2")
+
+with open(cwd / "ci" / "jenkins" / "Build.groovy.j2", "a") as f:
+f.write("test2")
+
+git.run("add", ".")
+git.run("commit", "-mtest")
+
+# Run generate.py in --check mode
+proc = subprocess.run(
+[
+str(script),
+"--check",
+"--base-ref",
+"HEAD~1",

Review Comment:
   sorry, just realized i'm looking at the test, ignore.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [tvm] areusch commented on a diff in pull request #12389: [ci] Check for timestamps in Jenkinsfile changes

2022-08-25 Thread GitBox


areusch commented on code in PR #12389:
URL: https://github.com/apache/tvm/pull/12389#discussion_r955435903


##
tests/python/ci/test_ci.py:
##
@@ -1100,5 +1101,72 @@ def test_should_rebuild_docker(tmpdir_factory, 
changed_files, name, check, expec
 assert proc.returncode == expected_code
 
 
+@tvm.testing.skip_if_wheel_test
+def test_jenkinsfile_timestamps(tmpdir_factory):
+"""
+Check that the timestamp added by ci/jenkins/generate.py is correctly
+checked with --check
+"""
+# Git repo set up
+git = TempGit(tmpdir_factory.mktemp("tmp_git_dir"))
+git.run("init")
+git.run("config", "user.name", "ci")
+git.run("config", "user.email", "em...@example.com")
+git.run("checkout", "-b", "main")
+git.run("remote", "add", "origin", "https://github.com/apache/tvm.git";)
+cwd = Path(git.cwd)
+
+# generate.py relies on files being at certain places in the repo, so copy
+# them all over
+shutil.copytree(REPO_ROOT / "ci", cwd / "ci")
+script = cwd / "ci" / "jenkins" / "generate.py"
+
+# generate the Jenkinsfile based on the copied templates as a starting 
point
+with open(cwd / "Jenkinsfile", "w") as f:
+f.write("abc")
+
+subprocess.run(
+[
+str(script),
+"--base-ref",
+"HEAD~1",
+],
+encoding="utf-8",
+stdout=subprocess.PIPE,
+cwd=git.cwd,
+check=True,
+)
+
+git.run("add", ".")
+git.run("commit", "-mtest")
+
+# Edit the Jenkinsfile and templates without updating the timestamp
+with open(cwd / "Jenkinsfile", "a") as f:
+f.write("test2")
+
+with open(cwd / "ci" / "jenkins" / "Build.groovy.j2", "a") as f:
+f.write("test2")
+
+git.run("add", ".")
+git.run("commit", "-mtest")
+
+# Run generate.py in --check mode
+proc = subprocess.run(
+[
+str(script),
+"--check",
+"--base-ref",
+"HEAD~1",

Review Comment:
   i think HEAD~1 won't catch everything right, since you could have multiple 
commits in a Jenkinsfile PR?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [tvm] areusch commented on a diff in pull request #12389: [ci] Check for timestamps in Jenkinsfile changes

2022-08-18 Thread GitBox


areusch commented on code in PR #12389:
URL: https://github.com/apache/tvm/pull/12389#discussion_r949597085


##
ci/jenkins/generate.py:
##
@@ -72,12 +75,49 @@
 }
 
 
-def lines_without_generated_tag(content):
+def lines_without_generated_tag(content: str) -> List[str]:
 return [
 line for line in content.splitlines(keepends=True) if not 
line.startswith("// Generated at")
 ]
 
 
+def changed_files() -> List[str]:
+proc = subprocess.run(
+["git", "diff", "origin/main", "HEAD", "--name-only"],

Review Comment:
   should we check against e.g. GIT_BRANCH to make this portable across 
different upstream branches?



##
ci/jenkins/generate.py:
##
@@ -72,12 +75,49 @@
 }
 
 
-def lines_without_generated_tag(content):
+def lines_without_generated_tag(content: str) -> List[str]:
 return [
 line for line in content.splitlines(keepends=True) if not 
line.startswith("// Generated at")
 ]
 
 
+def changed_files() -> List[str]:
+proc = subprocess.run(
+["git", "diff", "origin/main", "HEAD", "--name-only"],
+check=True,
+stdout=subprocess.PIPE,
+encoding="utf-8",
+)
+files = [f.strip() for f in proc.stdout.strip().split("\n")]
+files = [f for f in files if f != ""]
+return files
+
+
+def check_timestamp() -> None:
+"""
+Assert that the git diff against main contains a timestamp
+"""
+files = changed_files()
+
+# Check the Jenkinsfile timestamp if the templates were edited
+if any(fnmatch.fnmatch(f, "ci/jenkins/*.j2") for f in files):
+proc = subprocess.run(
+["git", "diff", "origin/main", "HEAD"],

Review Comment:
   shall we limit to Jenkinsfile?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@tvm.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org