scm: Fix bug when checking for valid revision and add tests.

When running `git rev-parse REV^{commit}` in Windows, `^` must be
escaped.
However, it was escaped more times than necessary.

Split IsValidRevision to call ResolveCommit, add tests to ResolveCommit,
and run scm.py tests on Windows.

Change-Id: I761a820394c8b5410d68b6ccd6c352c41c30c88c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/2092904
Reviewed-by: Josip Sokcevic <sokcevic@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

PRESUBMIT.py

@@ -79,7 +79,6 @@ def CommonChecks(input_api, output_api, tests_to_black_list, run_on_python3):
         r'.*ninjalog_uploader_test\.py$',
         r'.*recipes_test\.py$',
         r'.*roll_dep_test\.py$',
-        r'.*scm_unittest\.py$',
         r'.*subprocess2_test\.py$',
     ]
 

scm.py

@@ -375,25 +375,31 @@ class GIT(object):
     """Cleans up untracked file inside |relative_dir|."""
     return bool(GIT.Capture(['clean', '-df', relative_dir], cwd=cwd))
 
+  @staticmethod
+  def ResolveCommit(cwd, rev):
+    if sys.platform.startswith('win'):
+      # Windows .bat scripts use ^ as escape sequence, which means we have to
+      # escape it with itself for every .bat invocation.
+      needle = '%s^^{commit}' % rev
+    else:
+      needle = '%s^{commit}' % rev
+    try:
+      return GIT.Capture(['rev-parse', '--quiet', '--verify', needle], cwd=cwd)
+    except subprocess2.CalledProcessError:
+      return None
+
   @staticmethod
   def IsValidRevision(cwd, rev, sha_only=False):
     """Verifies the revision is a proper git revision.
 
     sha_only: Fail unless rev is a sha hash.
     """
-    if sys.platform.startswith('win'):
-      # Windows .bat scripts use ^ as escape sequence, which means we have to
-      # escape it with itself for every .bat invocation.
-      needle = '%s^^^^{commit}' % rev
-    else:
-      needle = '%s^{commit}' % rev
-    try:
-      sha = GIT.Capture(['rev-parse', '--verify', needle], cwd=cwd)
-      if sha_only:
-        return sha == rev.lower()
-      return True
-    except subprocess2.CalledProcessError:
+    sha = GIT.ResolveCommit(cwd, rev)
+    if sha is None:
       return False
+    if sha_only:
+      return sha == rev.lower()
+    return True
 
   @classmethod
   def AssertVersion(cls, min_version):

tests/scm_unittest.py

@@ -92,6 +92,14 @@ class RealGitTest(fake_repos.FakeReposTestBase):
     else:
       self.skipTest('git fake repos not available')
 
+  def testResolveCommit(self):
+    self.assertIsNone(scm.GIT.ResolveCommit(self.cwd, 'zebra'))
+    self.assertIsNone(scm.GIT.ResolveCommit(self.cwd, 'r123456'))
+    first_rev = self.githash('repo_1', 1)
+    self.assertEqual(first_rev, scm.GIT.ResolveCommit(self.cwd, first_rev))
+    self.assertEqual(
+        self.githash('repo_1', 2), scm.GIT.ResolveCommit(self.cwd, 'HEAD'))
+
   def testIsValidRevision(self):
     # Sha1's are [0-9a-z]{32}, so starting with a 'z' or 'r' should always fail.
     self.assertFalse(scm.GIT.IsValidRevision(cwd=self.cwd, rev='zebra'))