[gclient] Start actively using repo_path_map

Use repo_path_map in all places where patch_projects is currently used. Add gclient.get_repo_path function to resolve path to a repo given its URL. infra.git requires a non-trivial roll because of incorrect expectations. Recipe-Nontrivial-Roll: build Recipe-Nontrivial-Roll: infra Bug: 877161 Change-Id: Iaea0a3f14609bf0c18d4ecdf8564153b8052584d Reviewed-on: https://chromium-review.googlesource.com/1208363 Commit-Queue: Nodir Turakulov <nodir@chromium.org> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
7 years ago · 4e3920bef0
parent 9af7d107de
commit 4e3920bef0
18 changed files with 173 additions and 148 deletions
--- a/recipes/README.recipes.md
+++ b/recipes/README.recipes.md
@ -235,7 +235,7 @@ Returns (Path): The "depot_tools" root directory.
 &emsp; **@property**<br>&mdash; **def [upload\_to\_google\_storage\_path](/recipes/recipe_modules/depot_tools/api.py#17)(self):**
 ### *recipe_modules* / [gclient](/recipes/recipe_modules/gclient)

-[DEPS](/recipes/recipe_modules/gclient/__init__.py#1): [infra\_paths](#recipe_modules-infra_paths), [tryserver](#recipe_modules-tryserver), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/step][recipe_engine/recipe_modules/step]
+[DEPS](/recipes/recipe_modules/gclient/__init__.py#1): [gitiles](#recipe_modules-gitiles), [infra\_paths](#recipe_modules-infra_paths), [tryserver](#recipe_modules-tryserver), [recipe\_engine/context][recipe_engine/recipe_modules/context], [recipe\_engine/json][recipe_engine/recipe_modules/json], [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/platform][recipe_engine/recipe_modules/platform], [recipe\_engine/properties][recipe_engine/recipe_modules/properties], [recipe\_engine/python][recipe_engine/recipe_modules/python], [recipe\_engine/step][recipe_engine/recipe_modules/step]

 #### **class [GclientApi](/recipes/recipe_modules/gclient/api.py#65)([RecipeApi][recipe_engine/wkt/RecipeApi]):**

@ -248,10 +248,12 @@ Wrapper for easy calling of gclient steps.
 Remove all index.lock files. If a previous run of git crashed, bot was
 reset, etc... we might end up with leftover index.lock files.

-&mdash; **def [calculate\_patch\_root](/recipes/recipe_modules/gclient/api.py#305)(self, patch_project, gclient_config=None, patch_repo=None):**
+&mdash; **def [calculate\_patch\_root](/recipes/recipe_modules/gclient/api.py#339)(self, patch_project, gclient_config=None, patch_repo=None):**

 Returns path where a patch should be applied to based patch_project.

+TODO(nodir): delete this function in favor of get_repo_path.
+
 Maps the patch's repo to a path of directories relative to checkout's root,
 which describe where to place the patch. If no mapping is found for the
 repo url, falls back to trying to find a mapping for the old-style
@ -275,6 +277,14 @@ Return a step generator function for gclient checkouts.

 &mdash; **def [get\_config\_defaults](/recipes/recipe_modules/gclient/api.py#114)(self):**

+&mdash; **def [get\_repo\_path](/recipes/recipe_modules/gclient/api.py#309)(self, repo_url, gclient_config=None):**
+
+Returns local path to the repo checkout given its url.
+
+Consults cfg.repo_path_map and fallbacks to urls in configured solutions.
+
+Returns None if not found.
+
 &emsp; **@staticmethod**<br>&mdash; **def [got\_revision\_reverse\_mapping](/recipes/recipe_modules/gclient/api.py#125)(cfg):**

 Returns the merged got_revision_reverse_mapping.
@ -306,7 +316,7 @@ Chromium config. This may happen for one of two reasons:

 &mdash; **def [runhooks](/recipes/recipe_modules/gclient/api.py#264)(self, args=None, name='runhooks', \*\*kwargs):**

-&mdash; **def [set\_patch\_project\_revision](/recipes/recipe_modules/gclient/api.py#337)(self, patch_project, gclient_config=None):**
+&mdash; **def [set\_patch\_project\_revision](/recipes/recipe_modules/gclient/api.py#376)(self, patch_project, gclient_config=None):**

 Updates config revision corresponding to patch_project.

@ -539,6 +549,11 @@ DEPRECATED. Consider using gerrit.get_change_description instead.

 Module for polling a git repository using the Gitiles web interface.

+&mdash; **def [canonicalize\_repo\_url](/recipes/recipe_modules/gitiles/api.py#216)(self, repo_url):**
+
+Returns a canonical form of repo_url. If not recognized, returns as is.
+    
+
 &mdash; **def [commit\_log](/recipes/recipe_modules/gitiles/api.py#114)(self, url, commit, step_name=None, attempts=None):**

 Returns: (dict) the Gitiles commit log structure for a given commit.
@ -611,6 +626,10 @@ Returns (None, None) if repo_url is not recognized.
 &mdash; **def [refs](/recipes/recipe_modules/gitiles/api.py#56)(self, url, step_name='refs', attempts=None):**

 Returns a list of refs in the remote repository.
+
+&mdash; **def [unparse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#212)(self, host, project):**
+
+Generates a Gitiles repo URL. See also parse_repo_url.
 ### *recipe_modules* / [gsutil](/recipes/recipe_modules/gsutil)

 [DEPS](/recipes/recipe_modules/gsutil/__init__.py#1): [recipe\_engine/path][recipe_engine/recipe_modules/path], [recipe\_engine/python][recipe_engine/recipe_modules/python]
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_gerrit_webrtc.json
@ -51,7 +51,7 @@
      "--spec-path",
      "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]",
      "--patch_root",
-      "src/third_party/webrtc",
+      "src",
      "--revision_mapping_file",
      "{\"got_angle_revision\": \"src/third_party/angle\", \"got_cr_revision\": \"src\", \"got_revision\": \"src\", \"got_v8_revision\": \"src/v8\"}",
      "--git-cache-dir",
@ -61,7 +61,7 @@
      "--output_json",
      "/path/to/tmp/json",
      "--patch_ref",
-      "https://webrtc.googlesource.com/src@refs/changes/11/338811/3",
+      "https://chromium.googlesource.com/chromium/src@refs/changes/11/338811/3",
      "--revision",
      "src@HEAD",
      "--disable-syntax-validation"
@ -95,7 +95,7 @@
      "@@@STEP_LOG_LINE@json.output@    }@@@",
      "@@@STEP_LOG_LINE@json.output@  }, @@@",
      "@@@STEP_LOG_LINE@json.output@  \"patch_failure\": false, @@@",
-      "@@@STEP_LOG_LINE@json.output@  \"patch_root\": \"src/third_party/webrtc\", @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"patch_root\": \"src\", @@@",
      "@@@STEP_LOG_LINE@json.output@  \"properties\": {@@@",
      "@@@STEP_LOG_LINE@json.output@    \"got_angle_revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\", @@@",
      "@@@STEP_LOG_LINE@json.output@    \"got_angle_revision_cp\": \"refs/heads/master@{#297276}\", @@@",
@ -151,7 +151,7 @@
      "--spec-path",
      "cache_dir = '[GIT_CACHE]'\nsolutions = [{'deps_file': '.DEPS.git', 'managed': True, 'name': 'src', 'url': 'https://chromium.googlesource.com/chromium/src.git'}]",
      "--patch_root",
-      "src/third_party/webrtc",
+      "src",
      "--revision_mapping_file",
      "{\"got_angle_revision\": \"src/third_party/angle\", \"got_cr_revision\": \"src\", \"got_revision\": \"src\", \"got_v8_revision\": \"src/v8\"}",
      "--git-cache-dir",
@ -192,7 +192,7 @@
      "@@@STEP_LOG_LINE@json.output@    }@@@",
      "@@@STEP_LOG_LINE@json.output@  }, @@@",
      "@@@STEP_LOG_LINE@json.output@  \"patch_failure\": false, @@@",
-      "@@@STEP_LOG_LINE@json.output@  \"patch_root\": \"src/third_party/webrtc\", @@@",
+      "@@@STEP_LOG_LINE@json.output@  \"patch_root\": \"src\", @@@",
      "@@@STEP_LOG_LINE@json.output@  \"properties\": {@@@",
      "@@@STEP_LOG_LINE@json.output@    \"got_angle_revision\": \"fac9503c46405f77757b9a728eb85b8d7bc6080c\", @@@",
      "@@@STEP_LOG_LINE@json.output@    \"got_angle_revision_cp\": \"refs/heads/master@{#297276}\", @@@",
--- a/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8_head_by_default.json
+++ b/recipes/recipe_modules/bot_update/examples/full.expected/tryjob_v8_head_by_default.json
@ -61,7 +61,7 @@
      "--output_json",
      "/path/to/tmp/json",
      "--patch_ref",
-      "https://chromium.googlesource.com/chromium/src@refs/changes/89/456789/12",
+      "https://chromium.googlesource.com/v8/v8@refs/changes/89/456789/12",
      "--revision",
      "src@HEAD",
      "--revision",
--- a/recipes/recipe_modules/bot_update/examples/full.py
+++ b/recipes/recipe_modules/bot_update/examples/full.py
@ -35,8 +35,13 @@ def RunSteps(api):
  api.gclient.c.patch_projects['v8/v8'] = ('src/v8', 'HEAD')
  api.gclient.c.patch_projects['angle/angle'] = ('src/third_party/angle',
                                                 'HEAD')
-  api.gclient.c.repo_path_map['https://webrtc.googlesource.com/src'] = (
-      'src/third_party/webrtc', 'HEAD')
+  api.gclient.c.patch_projects['webrtc'] = ('src/third_party/webrtc', 'HEAD')
+  api.gclient.c.repo_path_map.update({
+      'https://chromium.googlesource.com/angle/angle': (
+          'src/third_party/angle', 'HEAD'),
+      'https://chromium.googlesource.com/v8/v8': ('src/v8', 'HEAD'),
+      'https://webrtc.googlesource.com/src': ('src/third_party/webrtc', 'HEAD'),
+  })

  patch = api.properties.get('patch', True)
  clobber = True if api.properties.get('clobber') else False
@ -144,21 +149,25 @@ def GenTests(api):
      gerrit_no_rebase_patch_ref=True
  )
  yield api.test('tryjob_v8') + api.properties(
+      repository='https://chromium.googlesource.com/v8/v8',
      issue=12345,
      patchset=654321,
      rietveld='https://rietveld.example.com/',
      patch_project='v8',
-      revisions={'src/v8': 'abc'}
+      revisions={'src/v8': 'abc'},
  )
  yield api.test('tryjob_v8_head_by_default') + api.properties.tryserver(
+      repository='https://chromium.googlesource.com/v8/v8',
      patch_project='v8',
  )
  yield api.test('tryjob_gerrit_angle') + api.properties.tryserver(
+      repository='https://chromium.googlesource.com/angle/angle',
      gerrit_project='angle/angle',
      patch_issue=338811,
      patch_set=3,
  )
  yield api.test('no_apply_patch_on_gclient') + api.properties.tryserver(
+      repository='https://chromium.googlesource.com/angle/angle',
      gerrit_project='angle/angle',
      patch_issue=338811,
      patch_set=3,
@ -166,11 +175,13 @@ def GenTests(api):
      apply_patch_on_gclient=False,
  )
  yield api.test('tryjob_gerrit_v8') + api.properties.tryserver(
+      repository='https://chromium.googlesource.com/v8/v8',
      gerrit_project='v8/v8',
      patch_issue=338811,
      patch_set=3,
  )
  yield api.test('tryjob_gerrit_v8_feature_branch') + api.properties.tryserver(
+      repository='https://chromium.googlesource.com/v8/v8',
      gerrit_project='v8/v8',
      patch_issue=338811,
      patch_set=3,
@ -180,6 +191,7 @@ def GenTests(api):
  )
  yield api.test('tryjob_gerrit_feature_branch') + api.properties.tryserver(
      buildername='feature_rel',
+      repository='https://chromium.googlesource.com/chromium/src',
      gerrit_project='chromium/src',
      patch_issue=338811,
      patch_set=3,
@ -188,6 +200,7 @@ def GenTests(api):
      api.gerrit.get_one_change_response_data(branch='experimental/feature'),
  )
  yield api.test('tryjob_gerrit_branch_heads') + api.properties.tryserver(
+      repository='https://chromium.googlesource.com/chromium/src',
      gerrit_project='chromium/src',
      patch_issue=338811,
      patch_set=3,
@ -196,6 +209,7 @@ def GenTests(api):
      api.gerrit.get_one_change_response_data(branch='refs/branch-heads/67'),
  )
  yield api.test('tryjob_gerrit_webrtc') + api.properties.tryserver(
+      repository='https://chromium.googlesource.com/chromium/src',
      gerrit_project='src',
      git_url='https://webrtc.googlesource.com/src',
      patch_issue=338811,
--- a/recipes/recipe_modules/gclient/init.py
+++ b/recipes/recipe_modules/gclient/init.py
@ -1,5 +1,6 @@
 DEPS = [
  'infra_paths',
+  'gitiles',
  'recipe_engine/context',
  'recipe_engine/json',
  'recipe_engine/path',
--- a/recipes/recipe_modules/gclient/api.py
+++ b/recipes/recipe_modules/gclient/api.py
@ -302,10 +302,46 @@ class GclientApi(recipe_api.RecipeApi):
      infra_step=True,
    )

+  def _canonicalize_repo_url(self, repo_url):
+    """Attempts to make repo_url canonical. Supports Gitiles URL."""
+    return self.m.gitiles.canonicalize_repo_url(repo_url)
+
+  def get_repo_path(self, repo_url, gclient_config=None):
+    """Returns local path to the repo checkout given its url.
+
+    Consults cfg.repo_path_map and fallbacks to urls in configured solutions.
+
+    Returns None if not found.
+    """
+    rel_path = self._get_repo_path(repo_url, gclient_config=gclient_config)
+    if rel_path:
+      return self.m.path.join(*rel_path.split('/'))
+    return None
+
+  def _get_repo_path(self, repo_url, gclient_config=None):
+    repo_url = self._canonicalize_repo_url(repo_url)
+    cfg = gclient_config or self.c
+    rel_path, _ = cfg.repo_path_map.get(repo_url, ('', ''))
+    if rel_path:
+      return rel_path
+
+    # repo_path_map keys may be non-canonical.
+    for key, (rel_path, _) in cfg.repo_path_map.iteritems():
+      if self._canonicalize_repo_url(key) == repo_url:
+        return rel_path
+
+    for s in cfg.solutions:
+      if self._canonicalize_repo_url(s.url) == repo_url:
+        return s.name
+
+    return None
+
  def calculate_patch_root(self, patch_project, gclient_config=None,
                           patch_repo=None):
    """Returns path where a patch should be applied to based patch_project.

+    TODO(nodir): delete this function in favor of get_repo_path.
+
    Maps the patch's repo to a path of directories relative to checkout's root,
    which describe where to place the patch. If no mapping is found for the
    repo url, falls back to trying to find a mapping for the old-style
@ -321,10 +357,13 @@ class GclientApi(recipe_api.RecipeApi):
      If patch_project is not given or not recognized, it'll be just first
      solution root.
    """
+    if patch_repo:
+      path = self.get_repo_path(patch_repo, gclient_config=gclient_config)
+      if path is not None:
+        return path
+
    cfg = gclient_config or self.c
-    root, _ = cfg.repo_path_map.get(patch_repo, ('', ''))
-    if not root:
-      root, _ = cfg.patch_projects.get(patch_project, ('', ''))
+    root, _ = cfg.patch_projects.get(patch_project, ('', ''))
    if not root:
      # Failure case - assume patch is for first solution, as this is what most
      # projects rely on.
--- a/recipes/recipe_modules/gclient/config.py
+++ b/recipes/recipe_modules/gclient/config.py
@ -82,6 +82,7 @@ def BaseConfig(USE_MIRROR=True, CACHE_DIR=None,
    #     'angle/angle': ('src/third_party/angle', 'HEAD')
    # then a patch to Angle project can be applied to a chromium src's
    # checkout after first updating Angle's repo to its master's HEAD.
+    # TODO(nodir): remove patch_projects in favor of repo_path_map.
    patch_projects = Dict(value_type=tuple, hidden=True),
    # Same as the above, except the keys are full repo URLs.
    repo_path_map = Dict(value_type=tuple, hidden=True),
@ -303,6 +304,16 @@ def infra(c):
  # TODO(phajdan.jr): remove recipes-py when it's not used for project name.
  p['infra/luci/recipes-py'] = ('infra/recipes-py', 'HEAD')
  p['recipe_engine'] = ('infra/recipes-py', 'HEAD')
+  c.repo_path_map.update({
+      'https://chromium.googlesource.com/infra/luci/gae': (
+          'infra/go/src/go.chromium.org/gae', 'HEAD'),
+      'https://chromium.googlesource.com/infra/luci/luci-py': (
+          'infra/luci', 'HEAD'),
+      'https://chromium.googlesource.com/infra/luci/luci-go': (
+          'infra/go/src/go.chromium.org/luci', 'HEAD'),
+      'https://chromium.googlesource.com/infra/luci/recipes-py': (
+          'infra/recipes-py', 'HEAD')
+  })

@config_ctx()
 def infra_internal(c):  # pragma: no cover
@ -338,7 +349,6 @@ def luci_py(c):
  # luci-py is checked out as part of infra just to have appengine
  # pre-installed, as that's what luci-py PRESUBMIT relies on.
  c.revisions['infra'] = 'origin/master'
-  # TODO(tandrii): make use of c.patch_projects.
  c.revisions['infra/luci'] = (
      gclient_api.RevisionFallbackChain('origin/master'))
  m = c.got_revision_mapping
@ -348,7 +358,6 @@ def luci_py(c):
@config_ctx(includes=['infra'])
 def recipes_py(c):
  c.revisions['infra'] = 'origin/master'
-  # TODO(tandrii): make use of c.patch_projects.
  c.revisions['infra/recipes-py'] = (
      gclient_api.RevisionFallbackChain('origin/master'))
  m = c.got_revision_mapping
--- a/recipes/recipe_modules/gclient/tests/patch_project.py
+++ b/recipes/recipe_modules/gclient/tests/patch_project.py
@ -25,11 +25,31 @@ def RunSteps(api, patch_project, patch_repository_url):
  soln.url = 'https://chromium.googlesource.com/chromium/src.git'
  src_cfg.patch_projects['v8'] = ('src/v8', 'HEAD')
  src_cfg.patch_projects['v8/v8'] = ('src/v8', 'HEAD')
-  src_cfg.repo_path_map['https://webrtc.googlesource.com'] = (
-      'src/third_party/webrtc', 'HEAD')
+  src_cfg.repo_path_map.update({
+      'https://chromium.googlesource.com/v8/v8': ('src/v8', 'HEAD'),
+      # non-canonical URL
+      'https://webrtc.googlesource.com/src.git': (
+          'src/third_party/webrtc', 'HEAD'),
+  })
+  assert api.gclient.get_repo_path(
+      'https://chromium.googlesource.com/chromium/src.git',
+      gclient_config=src_cfg) == 'src'
+  assert api.gclient.get_repo_path(
+      'https://chromium.googlesource.com/chromium/src',
+      gclient_config=src_cfg) == 'src'
+  assert api.gclient.get_repo_path(
+      'https://chromium.googlesource.com/v8/v8',
+      gclient_config=src_cfg) == 'src/v8'
+  assert api.gclient.get_repo_path(
+      'https://webrtc.googlesource.com/src',
+      gclient_config=src_cfg) == 'src/third_party/webrtc'
+  assert api.gclient.get_repo_path(
+      'https://example.googlesource.com/unrecognized',
+      gclient_config=src_cfg) is None
+
  api.gclient.c = src_cfg

-  patch_root = api.gclient.calculate_patch_root(
+  api.gclient.calculate_patch_root(
      patch_project, None, patch_repository_url)

  api.gclient.set_patch_project_revision(patch_project)
--- a/recipes/recipe_modules/gitiles/api.py
+++ b/recipes/recipe_modules/gitiles/api.py
@ -209,6 +209,19 @@ class Gitiles(recipe_api.RecipeApi):
    """
    return parse_repo_url(repo_url)

+  def unparse_repo_url(self, host, project):
+    """Generates a Gitiles repo URL. See also parse_repo_url."""
+    return unparse_repo_url(host, project)
+
+  def canonicalize_repo_url(self, repo_url):
+    """Returns a canonical form of repo_url. If not recognized, returns as is.
+    """
+    if repo_url:
+      host, project = parse_repo_url(repo_url)
+      if host and project:
+        repo_url = unparse_repo_url(host, project)
+    return repo_url
+

 def parse_http_host_and_path(url):
  # Copied from https://chromium.googlesource.com/infra/luci/recipes-py/+/809e57935211b3fcb802f74a7844d4f36eff6b87/recipe_modules/buildbucket/util.py
@ -238,3 +251,7 @@ def parse_repo_url(repo_url):
  if project.endswith('.git'):
    project = project[:-len('.git')]
  return host, project
+
+
+def unparse_repo_url(host, project):
+  return 'https://%s/%s' % (host, project)
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/a
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/a
@ -1,15 +0,0 @@
-[
-  {
-    "cmd": [
-      "echo",
-      "host",
-      "path/to/project"
-    ],
-    "name": "build"
-  },
-  {
-    "name": "$result",
-    "recipe_result": null,
-    "status_code": 0
-  }
-]
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/basic.json
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/basic.json
@ -1,12 +1,4 @@
 [
-  {
-    "cmd": [
-      "echo",
-      "host",
-      "path/to/project"
-    ],
-    "name": "build"
-  },
  {
    "name": "$result",
    "recipe_result": null,
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/git
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/git
@ -1,15 +0,0 @@
-[
-  {
-    "cmd": [
-      "echo",
-      "host",
-      "path/to/project"
-    ],
-    "name": "build"
-  },
-  {
-    "name": "$result",
-    "recipe_result": null,
-    "status_code": 0
-  }
-]
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/http
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/http
@ -1,15 +0,0 @@
-[
-  {
-    "cmd": [
-      "echo",
-      "host",
-      "path/to/project"
-    ],
-    "name": "build"
-  },
-  {
-    "name": "$result",
-    "recipe_result": null,
-    "status_code": 0
-  }
-]
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/http.json
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/http.json
@ -1,15 +0,0 @@
-[
-  {
-    "cmd": [
-      "echo",
-      "host",
-      "path/to/project"
-    ],
-    "name": "build"
-  },
-  {
-    "name": "$result",
-    "recipe_result": null,
-    "status_code": 0
-  }
-]
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/no
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/no
@ -1,15 +0,0 @@
-[
-  {
-    "cmd": [
-      "echo",
-      "host",
-      "path/to/project"
-    ],
-    "name": "build"
-  },
-  {
-    "name": "$result",
-    "recipe_result": null,
-    "status_code": 0
-  }
-]
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/plus.json
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/plus.json
@ -1,15 +0,0 @@
-[
-  {
-    "cmd": [
-      "echo",
-      "None",
-      "None"
-    ],
-    "name": "build"
-  },
-  {
-    "name": "$result",
-    "recipe_result": null,
-    "status_code": 0
-  }
-]
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/query
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.expected/query
@ -1,15 +0,0 @@
-[
-  {
-    "cmd": [
-      "echo",
-      "None",
-      "None"
-    ],
-    "name": "build"
-  },
-  {
-    "name": "$result",
-    "recipe_result": null,
-    "status_code": 0
-  }
-]
--- a/recipes/recipe_modules/gitiles/tests/parse_repo_url.py
+++ b/recipes/recipe_modules/gitiles/tests/parse_repo_url.py
@ -10,21 +10,40 @@ DEPS = [


 def RunSteps(api):
-  repo_url = api.properties['repo_url']
-  host, project = api.gitiles.parse_repo_url(repo_url)
-  api.step('build', ['echo', str(host), str(project)])
+  valid_urls = [
+    'https://host/path/to/project',
+    'http://host/path/to/project',
+    'https://host/a/path/to/project',
+    'https://host/path/to/project.git',
+    'http://host/a/path/to/project',
+    'host/a/path/to/project',
+  ]
+  for repo_url in valid_urls:
+    host, project = api.gitiles.parse_repo_url(repo_url)
+    assert host == 'host', host
+    assert project == 'path/to/project', project

+  invalid_urls = [
+    'https://host/a/path/to/project?a=b',
+    'https://host/path/to/project/+/master',
+  ]
+  for repo_url in invalid_urls:
+    host, project = api.gitiles.parse_repo_url(repo_url)
+    assert host is None
+    assert project is None

-def GenTests(api):
+  actual = api.gitiles.unparse_repo_url('host', 'path/to/project')
+  expected = 'https://host/path/to/project'
+  assert actual == expected
+
+  actual = api.gitiles.canonicalize_repo_url('http://host/path/to/project')
+  expected = 'https://host/path/to/project'
+  assert actual == expected
+
+  actual = api.gitiles.canonicalize_repo_url('http://unrecognized')
+  expected = 'http://unrecognized'
+  assert actual == expected

-  def case(name, repo_url):
-    return api.test(name) + api.properties(repo_url=repo_url)
-
-  yield case('basic', 'https://host/path/to/project')
-  yield case('http', 'http://host/path/to/project')
-  yield case('a prefix', 'https://host/a/path/to/project')
-  yield case('git suffix', 'https://host/path/to/project.git')
-  yield case('http and a prefix', 'http://host/a/path/to/project')
-  yield case('no scheme', 'host/a/path/to/project')
-  yield case('query string param', 'https://host/a/path/to/project?a=b')
-  yield case('plus', 'https://host/path/to/project/+/master')
+
+def GenTests(api):
+  yield api.test('basic')