Bug 1540306 [wpt PR 16110] - Don't use HEAD or FETCH_HEAD to checkout specific revisions. (#16066), a=testonly
authorjgraham <james@hoppipolla.co.uk>
Thu, 18 Apr 2019 11:56:56 +0000
changeset 529944 47b4de1dccba793969614a6d3be91c6dd46ba5f2
parent 529943 fce86a0833c29ae7da61bbb3c3f828ef0ffc94bd
child 529945 b7a6db8ce61ff3b1ef7916a9469d0104c6397965
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1540306, 16110, 16066
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1540306 [wpt PR 16110] - Don't use HEAD or FETCH_HEAD to checkout specific revisions. (#16066), a=testonly Automatic update from web-platform-tests Don't use HEAD or FETCH_HEAD to checkout specific revisions. (#16066) FETCH_HEAD is unreliable because it's a global variable that can be accidentially clobbered by adding an additional fetch anywhere in the pipeline. As a result running tests in CI has been broken since we chose the wrong revisions. HEAD is more reliable but doesn't exist until we first check something out. Instead, do the following: * Fetch the initial commits into a branch called task_head and check this out unconditionally. * For PRs, create branches called base_head and pr_head pointing to the two parents of the merge commit that we test on PRs. * Express all the other revisions in terms of task_head, pr_head and base_head, since they are both correct and more descriptive than using complex revision specifiers. * Move as much logic as possible out of the script baked in to the docker image since that's hardest to update. -- wpt-commits: e5044ace1ad217e683fb239ce6f88806f05139e3 wpt-pr: 16110
testing/web-platform/tests/.taskcluster.yml
testing/web-platform/tests/tools/ci/run_tc.py
testing/web-platform/tests/tools/ci/start.sh
testing/web-platform/tests/tools/docker/start.sh
--- a/testing/web-platform/tests/.taskcluster.yml
+++ b/testing/web-platform/tests/.taskcluster.yml
@@ -59,36 +59,36 @@ tasks:
                 name: wpt-${browser.name}-${browser.channel}-${chunk[0]}-${chunk[1]}
                 description: >-
                   A subset of WPT's "${chunk[0]}" tests (chunk number ${chunk[1]}
                   of ${chunk[2]}), run in the ${browser.channel} release of
                   ${browser.name}.
                 owner: ${event.pusher.email}
                 source: ${event.repository.url}
               payload:
-                image: harjgam/web-platform-tests:0.30
+                image: harjgam/web-platform-tests:0.32
                 maxRunTime: 7200
                 artifacts:
                   public/results:
                     path: /home/test/artifacts
                     type: directory
                 env:
                     TASK_EVENT: "${event_str}"
                 command:
                   - /bin/bash
                   - --login
                   - -c
                   - set -ex;
                     echo "wpt-${browser.name}-${browser.channel}-${chunk[0]}-${chunk[1]}";
                     ~/start.sh
                       ${event.repository.url}
-                      ${event.ref}
-                      ${event.after};
+                      ${event.ref};
                     cd ~/web-platform-tests;
                     ./tools/ci/run_tc.py
+                      --checkout=${event.after}
                       --oom-killer
                       --hosts
                       --browser=${browser.name}
                       --channel=${browser.channel}
                       --xvfb
                       run-all
                       ./tools/ci/taskcluster-run.py
                         ${browser.name}
@@ -107,35 +107,35 @@ tasks:
           # which should not trigger re-validation.
           $if: event.action in ['opened', 'reopened', 'synchronize']
           then:
             $map: [{name: firefox, channel: nightly}, {name: chrome, channel: dev}]
             each(browser):
               $map:
                 # This is the main place to define new stability checks
                 - name: wpt-${browser.name}-${browser.channel}-stability
-                  checkout: FETCH_HEAD
-                  diff_range: HEAD^
+                  checkout: task_head
+                  diff_base: base_head
                   description: >-
                     Verify that all tests affected by a pull request are stable
                     when executed in ${browser.name}.
                   extra_args: '--verify'
                 - name: wpt-${browser.name}-${browser.channel}-results
-                  checkout: FETCH_HEAD
-                  diff_range: HEAD^
+                  checkout: task_head
+                  diff_base: base_head
                   description: >-
                     Collect results for all tests affected by a pull request in
                     ${browser.name}.
                   extra_args: >-
                     --no-fail-on-unexpected
                     --log-wptreport=../artifacts/wpt_report.json
                     --log-wptscreenshot=../artifacts/wpt_screenshot.txt
                 - name: wpt-${browser.name}-${browser.channel}-results-without-changes
-                  checkout: FETCH_HEAD^
-                  diff_range: FETCH_HEAD
+                  checkout: base_head
+                  diff_base: task_head
                   description: >-
                     Collect results for all tests affected by a pull request in
                     ${browser.name} but without the changes in the PR.
                   extra_args: >-
                     --no-fail-on-unexpected
                     --log-wptreport=../artifacts/wpt_report.json
                     --log-wptscreenshot=../artifacts/wpt_screenshot.txt
               each(operation):
@@ -151,17 +151,17 @@ tasks:
                   else:
                     github-worker
                 metadata:
                   name: ${operation.name}
                   description: ${operation.description}
                   owner: ${event.pull_request.user.login}@users.noreply.github.com
                   source: ${event.repository.url}
                 payload:
-                  image: harjgam/web-platform-tests:0.30
+                  image: harjgam/web-platform-tests:0.32
                   maxRunTime: 7200
                   artifacts:
                     public/results:
                       path: /home/test/artifacts
                       type: directory
                   env:
                     TASK_EVENT: "${event_str}"
                   # Fetch the GitHub-provided merge commit (rather than the pull
@@ -173,28 +173,27 @@ tasks:
                   command:
                     - /bin/bash
                     - --login
                     - -c
                     - set -ex;
                       echo "${operation.name}";
                       ~/start.sh
                         ${event.repository.clone_url}
-                        refs/pull/${event.number}/merge
-                        FETCH_HEAD;
+                        refs/pull/${event.number}/merge;
                       cd web-platform-tests;
                       ./tools/ci/run_tc.py
                         --checkout=${operation.checkout}
                         --oom-killer
                         --browser=${browser.name}
                         --channel=${browser.channel}
                         --xvfb
                         stability
                         ./tools/ci/taskcluster-run.py
-                          --commit-range ${operation.diff_range}
+                          --commit-range ${operation.diff_base}
                           ${browser.name}
                           --
                           --channel=${browser.channel}
                           ${operation.extra_args};
       - $map:
           # This is the main point to define new CI checks other than stability checks
         - name: lint
           description: >-
@@ -305,28 +304,27 @@ tasks:
                   else:
                     github-worker
                 metadata:
                   name: ${operation.name}
                   description: ${operation.description}
                   owner: ${event.sender.login}@users.noreply.github.com
                   source: ${event.repository.url}
                 payload:
-                  image: harjgam/web-platform-tests:0.30
+                  image: harjgam/web-platform-tests:0.32
                   maxRunTime: 7200
                   artifacts:
                     public/results:
                       path: /home/test/artifacts
                       type: directory
                   env:
                     TASK_EVENT: "${event_str}"
                   command:
                     - /bin/bash
                     - --login
                     - -c
                     - set -ex;
                       echo "${operation.name}";
                       ~/start.sh
                         ${event.repository.clone_url}
-                        ${checkout_ref}
-                        FETCH_HEAD;
+                        ${checkout_ref};
                       cd ~/web-platform-tests;
                       ${operation.script};
--- a/testing/web-platform/tests/tools/ci/run_tc.py
+++ b/testing/web-platform/tests/tools/ci/run_tc.py
@@ -112,17 +112,17 @@ def start_userspace_oom_killer():
     start(["sudo", "earlyoom", "-p", "-r", "60" "--prefer=(chrome|firefox)", "--avoid=python"])
 
 
 def make_hosts_file():
     subprocess.check_call(["sudo", "sh", "-c", "./wpt make-hosts-file >> /etc/hosts"])
 
 
 def checkout_revision(rev):
-    subprocess.check_call(["git", "checkout", "-q", rev])
+    subprocess.check_call(["git", "checkout", "--quiet", rev])
 
 
 def install_chrome(channel):
     if channel in ("experimental", "dev", "nightly"):
         deb_archive = "google-chrome-unstable_current_amd64.deb"
     elif channel == "beta":
         deb_archive = "google-chrome-beta_current_amd64.deb"
     elif channel == "stable":
@@ -130,18 +130,18 @@ def install_chrome(channel):
     else:
         raise ValueError("Unrecognized release channel: %s" % channel)
 
     dest = os.path.join("/tmp", deb_archive)
     resp = urlopen("https://dl.google.com/linux/direct/%s" % deb_archive)
     with open(dest, "w") as f:
         f.write(resp.read())
 
-    subprocess.check_call(["sudo", "apt-get", "-qqy", "update"])
-    subprocess.check_call(["sudo", "gdebi", "-n", "/tmp/%s" % deb_archive])
+    run(["sudo", "apt-get", "-qqy", "update"])
+    run(["sudo", "gdebi", "-qn", "/tmp/%s" % deb_archive])
 
 
 def start_xvfb():
     start(["sudo", "Xvfb", os.environ["DISPLAY"], "-screen", "0",
            "%sx%sx%s" % (os.environ["SCREEN_WIDTH"],
                          os.environ["SCREEN_HEIGHT"],
                          os.environ["SCREEN_DEPTH"])])
     start(["sudo", "fluxbox", "-display", os.environ["DISPLAY"]])
@@ -216,33 +216,50 @@ def setup_environment(args):
 
     if args.oom_killer:
         start_userspace_oom_killer()
 
     if args.checkout:
         checkout_revision(args.checkout)
 
 
+def setup_repository():
+    if os.environ.get("GITHUB_PULL_REQUEST", "false") != "false":
+        parents = run(["git", "show", "--format=%P", "task_head"], return_stdout=True).strip().split()
+        if len(parents) == 2:
+            base_head = parents[0]
+            pr_head = parents[1]
+
+            run(["git", "branch", "base_head", base_head])
+            run(["git", "branch", "pr_head", pr_head])
+        else:
+            print("ERROR: Pull request HEAD wasn't a 2-parent merge commit; "
+                  "expected to test the merge of PR into the base")
+            sys.exit(1)
+
+    branch = os.environ.get("GITHUB_BRANCH")
+    if branch:
+        # Ensure that the remote base branch exists
+        # TODO: move this somewhere earlier in the task
+        run(["git", "fetch", "--quiet", "origin", "%s:%s" % (branch, branch)])
+
+
 def main():
     args = get_parser().parse_args()
     try:
         event = json.loads(os.environ["TASK_EVENT"])
     except KeyError:
         print("WARNING: Missing TASK_EVENT environment variable")
         # For example under local testing
         event = {}
 
     if event:
         set_variables(event)
 
-    if os.environ.get("GITHUB_BRANCH"):
-        # Ensure that the remote base branch exists
-        # TODO: move this somewhere earlier in the task
-        run(["git", "fetch", "origin", "%s:%s" % (os.environ["GITHUB_BRANCH"],
-                                                  os.environ["GITHUB_BRANCH"])])
+    setup_repository()
 
     extra_jobs = get_extra_jobs(event)
 
     job = args.job
 
     run_if = [(lambda: job == "all", "job set to 'all'"),
               (lambda:"all" in extra_jobs, "Manually specified jobs includes 'all'"),
               (lambda:job in extra_jobs, "Manually specified jobs includes '%s'" % job),
@@ -255,14 +272,14 @@ def main():
     else:
         print("Job not scheduled for this push")
         return
 
     # Run the job
     setup_environment(args)
     os.chdir(root)
     cmd = [args.script] + args.script_args
-    print(cmd)
+    print(" ".join(cmd))
     sys.exit(subprocess.call(cmd))
 
 
 if __name__ == "__main__":
     main()
deleted file mode 100644
--- a/testing/web-platform/tests/tools/ci/start.sh
+++ /dev/null
@@ -1,1 +0,0 @@
-# Contents of this script superceeded by tools/ci/run_tc.py
--- a/testing/web-platform/tests/tools/docker/start.sh
+++ b/testing/web-platform/tests/tools/docker/start.sh
@@ -9,33 +9,21 @@
 #   docker push <tag>
 # Update the `image` specified in the project's .taskcluster.yml file
 
 
 set -ex
 
 REMOTE=${1:-https://github.com/web-platform-tests/wpt}
 REF=${2:-master}
-REVISION=${3:-FETCH_HEAD}
-BROWSER=${4:-all}
-CHANNEL=${5:-nightly}
 
 cd ~
 
 mkdir web-platform-tests
 cd web-platform-tests
 
 git init
 git remote add origin ${REMOTE}
 
 # Initially we just fetch 50 commits in order to save several minutes of fetching
-retry git fetch --quiet --depth=50 --tags origin ${REF}
+retry git fetch --quiet --depth=50 --tags origin ${REF}:task_head
 
-if [[ ! `git rev-parse --verify -q ${REVISION}` ]];
-then
-    # But if for some reason the commit under test isn't in that range, we give in and
-    # fetch everything
-    retry git fetch -q --unshallow ${REMOTE}
-    git rev-parse --verify ${REVISION}
-fi
-git checkout -b build ${REVISION}
-
-source tools/ci/start.sh
+git checkout --quiet task_head