gclient: Make smoketests run on Windows.

Bug: 1024683
Change-Id: I1c30473699c1bd6b198188b53b632e3617fed335
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1929653
Reviewed-by: Anthony Polito <apolito@google.com>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

PRESUBMIT.py

@@ -70,7 +70,6 @@ def CommonChecks(input_api, output_api, tests_to_black_list, run_on_python3):
     print('Warning: skipping most unit tests on Windows')
     tests_to_black_list = [
         r'.*auth_test\.py$',
-        r'.*gclient_smoketest\.py$',
         r'.*git_cl_test\.py$',
         r'.*git_common_test\.py$',
         r'.*git_hyper_blame_test\.py$',

fix_encoding.py

@@ -84,6 +84,10 @@ def fix_win_sys_argv(encoding):
   if _SYS_ARGV_PROCESSED:
     return False
 
+  if sys.version_info.major == 3:
+    _SYS_ARGV_PROCESSED = True
+    return True
+
   # These types are available on linux but not Mac.
   # pylint: disable=no-name-in-module,F0401
   from ctypes import byref, c_int, POINTER, windll, WINFUNCTYPE
@@ -269,6 +273,9 @@ class WinUnicodeOutput(WinUnicodeOutputBase):
       if sys.version_info.major == 2 and isinstance(text, unicode):
         # Replace characters that cannot be printed instead of failing.
         text = text.encode(self.encoding, 'replace')
+      if sys.version_info.major == 3 and isinstance(text, bytes):
+        # Replace characters that cannot be printed instead of failing.
+        text = text.decode(self.encoding, 'replace')
       self._stream.write(text)
     except Exception as e:
       complain('%s.write: %r' % (self.name, e))

gclient.py

@@ -1042,8 +1042,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
       if isinstance(value, basestring):
         value = gclient_eval.EvaluateCondition(value, variables)
       lines.append('%s = %s' % (arg, ToGNString(value)))
-    with open(os.path.join(self.root.root_dir, self._gn_args_file), 'w') as f:
+    with open(os.path.join(self.root.root_dir, self._gn_args_file), 'wb') as f:
-      f.write('\n'.join(lines))
+      f.write('\n'.join(lines).encode('utf-8', 'replace'))
 
   @gclient_utils.lockedmethod
   def _run_is_done(self, file_list):
@@ -1322,10 +1322,10 @@ class GClient(GitDependency):
 
   DEFAULT_CLIENT_FILE_TEXT = ("""\
 solutions = [
-  { "name"        : "%(solution_name)s",
+  { "name"        : %(solution_name)r,
-    "url"         : "%(solution_url)s",
+    "url"         : %(solution_url)r,
-    "deps_file"   : "%(deps_file)s",
+    "deps_file"   : %(deps_file)r,
-    "managed"     : %(managed)s,
+    "managed"     : %(managed)r,
     "custom_deps" : {
     },
     "custom_vars": %(custom_vars)r,

gclient_scm.py

@@ -1475,15 +1475,16 @@ class CipdRoot(object):
   @contextlib.contextmanager
   def _create_ensure_file(self):
     try:
-      ensure_file = None
+      contents = '$ParanoidMode CheckPresence\n\n'
-      with tempfile.NamedTemporaryFile(
-          suffix='.ensure', delete=False, mode='w') as ensure_file:
-        ensure_file.write('$ParanoidMode CheckPresence\n\n')
       for subdir, packages in sorted(self._packages_by_subdir.items()):
-          ensure_file.write('@Subdir %s\n' % subdir)
+        contents += '@Subdir %s\n' % subdir
         for package in sorted(packages, key=lambda p: p.name):
-            ensure_file.write('%s %s\n' % (package.name, package.version))
+          contents += '%s %s\n' % (package.name, package.version)
-          ensure_file.write('\n')
+        contents += '\n'
+      ensure_file = None
+      with tempfile.NamedTemporaryFile(
+          suffix='.ensure', delete=False, mode='wb') as ensure_file:
+        ensure_file.write(contents.encode('utf-8', 'replace'))
       yield ensure_file.name
     finally:
       if ensure_file is not None and os.path.exists(ensure_file.name):

testing_support/fake_cipd.py

@@ -40,8 +40,8 @@ def main():
   for path, packages in new_content.items():
     if not os.path.exists(path):
       os.makedirs(path)
-    with open(os.path.join(path, '_cipd'), 'w') as f:
+    with open(os.path.join(path, '_cipd'), 'wb') as f:
-      f.write('\n'.join(packages))
+      f.write('\n'.join(packages).encode('utf-8', 'replace'))
 
   # Save the ensure file that we got
   shutil.copy(args.ensure_file, os.path.join(args.root, '_cipd'))

testing_support/fake_repos.py

@@ -249,7 +249,7 @@ gclient_gn_args = [
 ]
 deps = {
   'src/repo2': {
-    'url': '%(git_base)srepo_2',
+    'url': %(git_base)r + 'repo_2',
     'condition': 'True',
   },
   'src/repo2/repo3': '/' + Var('DummyVariable') + '_3@%(hash3)s',
@@ -305,7 +305,7 @@ deps = {
     self._commit_git('repo_1', {
       'DEPS': """
 deps = {
-  'src/repo2': '%(git_base)srepo_2@%(hash)s',
+  'src/repo2': %(git_base)r + 'repo_2@%(hash)s',
   'src/repo2/repo_renamed': '/repo_3',
   'src/should_not_process': {
     'url': '/repo_4',
@@ -341,8 +341,8 @@ hooks = [
     self._commit_git('repo_5', {
       'DEPS': """
 deps = {
-  'src/repo1': '%(git_base)srepo_1@%(hash1)s',
+  'src/repo1': %(git_base)r + 'repo_1@%(hash1)s',
-  'src/repo2': '%(git_base)srepo_2@%(hash2)s',
+  'src/repo2': %(git_base)r + 'repo_2@%(hash2)s',
 }
 
 # Hooks to run after a project is processed but before its dependencies are
@@ -363,8 +363,8 @@ pre_deps_hooks = [
     self._commit_git('repo_5', {
       'DEPS': """
 deps = {
-  'src/repo1': '%(git_base)srepo_1@%(hash1)s',
+  'src/repo1': %(git_base)r + 'repo_1@%(hash1)s',
-  'src/repo2': '%(git_base)srepo_2@%(hash2)s',
+  'src/repo2': %(git_base)r + 'repo_2@%(hash2)s',
 }
 
 # Hooks to run after a project is processed but before its dependencies are
@@ -390,7 +390,7 @@ pre_deps_hooks = [
       'DEPS': """
 vars = {
   'DummyVariable': 'repo',
-  'git_base': '%(git_base)s',
+  'git_base': %(git_base)r,
   'hook1_contents': 'git_hooked1',
   'repo5_var': '/repo_5',
 
@@ -413,7 +413,7 @@ gclient_gn_args = [
 ]
 
 allowed_hosts = [
-  '%(git_base)s',
+  %(git_base)r,
 ]
 deps = {
   'src/repo2': {
@@ -718,13 +718,13 @@ class FakeRepoSkiaDEPS(FakeReposBase):
   NB_GIT_REPOS = 5
 
   DEPS_git_pre = """deps = {
-  'src/third_party/skia/gyp': '%(git_base)srepo_3',
+  'src/third_party/skia/gyp': %(git_base)r + 'repo_3',
-  'src/third_party/skia/include': '%(git_base)srepo_4',
+  'src/third_party/skia/include': %(git_base)r + 'repo_4',
-  'src/third_party/skia/src': '%(git_base)srepo_5',
+  'src/third_party/skia/src': %(git_base)r + 'repo_5',
 }"""
 
   DEPS_post = """deps = {
-  'src/third_party/skia': '%(git_base)srepo_1',
+  'src/third_party/skia': %(git_base)r + 'repo_1',
 }"""
 
   def populateGit(self):

tests/gclient_smoketest.py

@@ -40,6 +40,7 @@ class GClientSmokeBase(fake_repos.FakeReposTestBase):
     self.env['DEPOT_TOOLS_METRICS'] = '0'
     # Suppress Python 3 warnings and other test undesirables.
     self.env['GCLIENT_TEST'] = '1'
+    self.maxDiff = None
 
   def gclient(self, cmd, cwd=None, error_ok=False):
     if not cwd:
@@ -193,75 +194,75 @@ class GClientSmoke(GClientSmokeBase):
       self.check(('', '', 0), results)
       mode = 'r' if sys.version_info.major == 3 else 'rU'
       with open(p, mode) as f:
-        self.checkString(expected, f.read())
+        actual = {}
-
+        exec(f.read(), {}, actual)
-    test(['config', self.git_base + 'src/'],
+        self.assertEqual(expected, actual)
-         ('solutions = [\n'
+
-          '  { "name"        : "src",\n'
+    test(
-          '    "url"         : "%ssrc",\n'
+        ['config', self.git_base + 'src/'],
-          '    "deps_file"   : "DEPS",\n'
+        {
-          '    "managed"     : True,\n'
+            'solutions': [{
-          '    "custom_deps" : {\n'
+                'name': 'src',
-          '    },\n'
+                'url': self.git_base + 'src',
-          '    "custom_vars": {},\n'
+                'deps_file': 'DEPS',
-          '  },\n'
+                'managed': True,
-          ']\n' % self.git_base))
+                'custom_deps': {},
-
+                'custom_vars': {},
-    test(['config', self.git_base + 'repo_1', '--name', 'src',
+            }],
+        })
+
+    test(['config', self.git_base + 'repo_1',
+          '--name', 'src',
           '--cache-dir', 'none'],
-         ('solutions = [\n'
+         {'solutions': [{
-          '  { "name"        : "src",\n'
+             'name': 'src',
-          '    "url"         : "%srepo_1",\n'
+             'url': self.git_base + 'repo_1',
-          '    "deps_file"   : "DEPS",\n'
+             'deps_file': 'DEPS',
-          '    "managed"     : True,\n'
+             'managed': True,
-          '    "custom_deps" : {\n'
+             'custom_deps': {},
-          '    },\n'
+             'custom_vars': {},
-          '    "custom_vars": {},\n'
+          }],
-          '  },\n'
+          'cache_dir': None})
-          ']\n'
-          'cache_dir = None\n') % self.git_base)
 
     test(['config', 'https://example.com/foo', 'faa',
           '--cache-dir', 'something'],
-         'solutions = [\n'
+         {'solutions': [{
-         '  { "name"        : "foo",\n'
+             'name': 'foo',
-         '    "url"         : "https://example.com/foo",\n'
+             'url': 'https://example.com/foo',
-         '    "deps_file"   : "DEPS",\n'
+             'deps_file': 'DEPS',
-         '    "managed"     : True,\n'
+             'managed': True,
-         '    "custom_deps" : {\n'
+             'custom_deps': {},
-         '    },\n'
+             'custom_vars': {},
-         '    "custom_vars": {},\n'
+          }],
-         '  },\n'
+          'cache_dir': 'something'})
-         ']\n'
+
-         'cache_dir = \'something\'\n')
+    test(['config', 'https://example.com/foo',
-
+          '--deps', 'blah'],
-    test(['config', 'https://example.com/foo', '--deps', 'blah'],
+         {'solutions': [{
-         'solutions = [\n'
+             'name': 'foo',
-         '  { "name"        : "foo",\n'
+             'url': 'https://example.com/foo',
-         '    "url"         : "https://example.com/foo",\n'
+             'deps_file': 'blah',
-         '    "deps_file"   : "blah",\n'
+             'managed': True,
-         '    "managed"     : True,\n'
+             'custom_deps': {},
-         '    "custom_deps" : {\n'
+             'custom_vars': {},
-         '    },\n'
+          }]})
-         '    "custom_vars": {},\n'
-         '  },\n'
-         ']\n')
 
     test(['config', self.git_base + 'src/',
           '--custom-var', 'bool_var=True',
           '--custom-var', 'str_var="abc"'],
-         ('solutions = [\n'
+         {'solutions': [{
-          '  { "name"        : "src",\n'
+             'name': 'src',
-          '    "url"         : "%ssrc",\n'
+             'url': self.git_base + 'src',
-          '    "deps_file"   : "DEPS",\n'
+             'deps_file': 'DEPS',
-          '    "managed"     : True,\n'
+             'managed': True,
-          '    "custom_deps" : {\n'
+             'custom_deps': {},
-          '    },\n'
+             'custom_vars': {
-          '    "custom_vars": {\'bool_var\': True, \'str_var\': \'abc\'},\n'
+                 'bool_var': True,
-          '  },\n'
+                 'str_var': 'abc',
-          ']\n') % self.git_base)
+             },
-
+          }]})
-    test(['config', '--spec', '["blah blah"]'], '["blah blah"]')
+
+    test(['config', '--spec', 'bah = ["blah blah"]'], {'bah': ["blah blah"]})
 
     os.remove(p)
     results = self.gclient(['config', 'foo', 'faa', 'fuu'], error_ok=True)
@@ -314,10 +315,10 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.env['PATH'] = (os.path.join(ROOT_DIR, 'testing_support')
                         + os.pathsep + self.env['PATH'])
     self.enabled = self.FAKE_REPOS.set_up_git()
+    if not self.enabled:
+      self.skipTest('git fake repos not available')
 
   def testSync(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     # Test unversioned checkout.
     self.parseGclient(
@@ -386,7 +387,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     with open(output_json) as f:
       output_json = json.load(f)
 
-    self.maxDiff = None
     out = {
         'solutions': {
             'src/': {
@@ -420,8 +420,6 @@ class GClientSmokeGIT(GClientSmokeBase):
 
   def testSyncIgnoredSolutionName(self):
     """TODO(maruel): This will become an error soon."""
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.parseGclient(
         ['sync', '--deps', 'mac', '--jobs', '1',
@@ -437,8 +435,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertTree(tree)
 
   def testSyncNoSolutionName(self):
-    if not self.enabled:
-      return
     # When no solution name is provided, gclient uses the first solution listed.
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.parseGclient(
@@ -461,8 +457,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertTree(tree)
 
   def testSyncJobs(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     # Test unversioned checkout.
     self.parseGclient(
@@ -528,15 +522,11 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertTree(tree)
 
   def testSyncFetch(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_13', '--name', 'src'])
     self.gclient(
         ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 2)])
 
   def testSyncFetchUpdate(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_13', '--name', 'src'])
 
     # Sync to an earlier revision first, one that doesn't refer to
@@ -549,18 +539,14 @@ class GClientSmokeGIT(GClientSmokeBase):
         ['sync', '-v', '-v', '-v', '--revision', self.githash('repo_13', 2)])
 
   def testSyncDirect(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_12', '--name', 'src'])
     self.gclient(
         ['sync', '-v', '-v', '-v', '--revision', 'refs/changes/1212'])
 
   def testSyncUnmanaged(self):
-    if not self.enabled:
-      return
     self.gclient([
         'config', '--spec',
-        'solutions=[{"name":"src", "url": "%s", "managed": False}]' % (
+        'solutions=[{"name":"src", "url": %r, "managed": False}]' % (
             self.git_base + 'repo_5')])
     self.gclient([
         'sync', '--revision', 'src@' + self.githash('repo_5', 2)])
@@ -572,12 +558,9 @@ class GClientSmokeGIT(GClientSmokeBase):
                                 ('repo_1@1', 'src/repo1'),
                                 ('repo_2@1', 'src/repo2'))
     tree['src/git_pre_deps_hooked'] = 'git_pre_deps_hooked'
-    self.maxDiff = None
     self.assertTree(tree)
 
   def testSyncUrl(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient([
         'sync', '-v', '-v', '-v',
@@ -594,8 +577,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertTree(tree)
 
   def testSyncPatchRef(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient([
         'sync', '-v', '-v', '-v',
@@ -618,8 +599,6 @@ class GClientSmokeGIT(GClientSmokeBase):
         self.gitrevparse(os.path.join(self.root_dir, 'src/repo2')))
 
   def testSyncPatchRefNoHooks(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient([
         'sync', '-v', '-v', '-v',
@@ -641,8 +620,6 @@ class GClientSmokeGIT(GClientSmokeBase):
         self.gitrevparse(os.path.join(self.root_dir, 'src/repo2')))
 
   def testRunHooks(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     tree = self.mangle_git_tree(('repo_1@2', 'src'),
@@ -667,8 +644,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertTree(tree)
 
   def testRunHooksCondition(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_7', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     tree = self.mangle_git_tree(('repo_7@1', 'src'))
@@ -676,8 +651,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertTree(tree)
 
   def testPreDepsHooks(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_5', '--name', 'src'])
     expectation = [
         ('running', self.root_dir),                 # git clone
@@ -728,17 +701,16 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertTree(tree)
 
   def testPreDepsHooksError(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_5', '--name', 'src'])
     expectated_stdout = [
         ('running', self.root_dir),                 # git clone
         ('running', self.root_dir),                 # pre-deps hook
         ('running', self.root_dir),                 # pre-deps hook (fails)
     ]
-    expected_stderr = ("Error: Command 'vpython -c import sys; "
+    vpython = 'vpython.bat' if sys.platform == 'win32' else 'vpython'
+    expected_stderr = ("Error: Command '%s -c import sys; "
                        "sys.exit(1)' returned non-zero exit status 1 in %s\n"
-                       % self.root_dir)
+                       % (vpython, self.root_dir))
     stdout, stderr, retcode = self.gclient(
         ['sync', '--deps', 'mac', '--jobs=1', '--revision',
          'src@' + self.githash('repo_5', 3)], error_ok=True)
@@ -747,8 +719,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.checkBlock(stdout, expectated_stdout)
 
   def testRevInfo(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     results = self.gclient(['revinfo', '--deps', 'mac'])
@@ -762,8 +732,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.check((out, '', 0), results)
 
   def testRevInfoActual(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     results = self.gclient(['revinfo', '--deps', 'mac', '--actual'])
@@ -779,8 +747,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.check((out, '', 0), results)
 
   def testRevInfoFilterPath(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     results = self.gclient(['revinfo', '--deps', 'mac', '--filter', 'src'])
@@ -791,8 +757,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.check((out, '', 0), results)
 
   def testRevInfoFilterURL(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     results = self.gclient(['revinfo', '--deps', 'mac',
@@ -805,8 +769,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.check((out, '', 0), results)
 
   def testRevInfoFilterURLOrPath(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     results = self.gclient(['revinfo', '--deps', 'mac', '--filter', 'src',
@@ -820,8 +782,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.check((out, '', 0), results)
 
   def testRevInfoJsonOutput(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     output_json = os.path.join(self.root_dir, 'output.json')
@@ -846,8 +806,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     self.assertEqual(out, output_json)
 
   def testRevInfoJsonOutputSnapshot(self):
-    if not self.enabled:
-      return
     self.gclient(['config', self.git_base + 'repo_1', '--name', 'src'])
     self.gclient(['sync', '--deps', 'mac'])
     output_json = os.path.join(self.root_dir, 'output.json')
@@ -1015,10 +973,9 @@ class GClientSmokeGIT(GClientSmokeBase):
         'bar_rev',
     ], results[0].splitlines())
 
+  # TODO(crbug.com/1024683): Enable for windows.
+  @unittest.skipIf(sys.platform == 'win32', 'not yet fixed on win')
   def testFlatten(self):
-    if not self.enabled:
-      return
-
     output_deps = os.path.join(self.root_dir, 'DEPS.flattened')
     self.assertFalse(os.path.exists(output_deps))
 
@@ -1045,7 +1002,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     with open(output_deps) as f:
       deps_contents = f.read()
 
-    self.maxDiff = None  # pylint: disable=attribute-defined-outside-init
     self.assertEqual([
         'gclient_gn_args_file = "src/repo2/gclient.args"',
         'gclient_gn_args = [\'false_var\', \'false_str_var\', \'true_var\', '
@@ -1227,10 +1183,9 @@ class GClientSmokeGIT(GClientSmokeBase):
         '# ' + self.git_base + 'repo_8, DEPS',
     ], deps_contents.splitlines())
 
+  # TODO(crbug.com/1024683): Enable for windows.
+  @unittest.skipIf(sys.platform == 'win32', 'not yet fixed on win')
   def testFlattenPinAllDeps(self):
-    if not self.enabled:
-      return
-
     output_deps = os.path.join(self.root_dir, 'DEPS.flattened')
     self.assertFalse(os.path.exists(output_deps))
 
@@ -1242,7 +1197,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     with open(output_deps) as f:
       deps_contents = f.read()
 
-    self.maxDiff = None  # pylint: disable=attribute-defined-outside-init
     self.assertEqual([
         'gclient_gn_args_file = "src/repo2/gclient.args"',
         'gclient_gn_args = [\'false_var\', \'false_str_var\', \'true_var\', '
@@ -1438,10 +1392,9 @@ class GClientSmokeGIT(GClientSmokeBase):
             self.githash('repo_8', 1)),
     ], deps_contents.splitlines())
 
+  # TODO(crbug.com/1024683): Enable for windows.
+  @unittest.skipIf(sys.platform == 'win32', 'not yet fixed on win')
   def testFlattenRecursedeps(self):
-    if not self.enabled:
-      return
-
     output_deps = os.path.join(self.root_dir, 'DEPS.flattened')
     self.assertFalse(os.path.exists(output_deps))
 
@@ -1457,7 +1410,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     with open(output_deps) as f:
       deps_contents = f.read()
 
-    self.maxDiff = None
     self.assertEqual([
         'gclient_gn_args_file = "src/repo8/gclient.args"',
         "gclient_gn_args = ['str_var']",
@@ -1543,10 +1495,9 @@ class GClientSmokeGIT(GClientSmokeBase):
                      ['src/repo9', self.git_base + 'repo_9']]},
     ], deps_files_contents)
 
+  # TODO(crbug.com/1024683): Enable for windows.
+  @unittest.skipIf(sys.platform == 'win32', 'not yet fixed on win')
   def testFlattenCipd(self):
-    if not self.enabled:
-      return
-
     output_deps = os.path.join(self.root_dir, 'DEPS.flattened')
     self.assertFalse(os.path.exists(output_deps))
 
@@ -1557,7 +1508,6 @@ class GClientSmokeGIT(GClientSmokeBase):
     with open(output_deps) as f:
       deps_contents = f.read()
 
-    self.maxDiff = None  # pylint: disable=attribute-defined-outside-init
     self.assertEqual([
         'deps = {',
         '  # src',
@@ -1613,11 +1563,12 @@ class GClientSmokeGITMutates(GClientSmokeBase):
   def setUp(self):
     super(GClientSmokeGITMutates, self).setUp()
     self.enabled = self.FAKE_REPOS.set_up_git()
-
-  def testRevertAndStatus(self):
     if not self.enabled:
-      return
+      self.skipTest('git fake repos not available')
 
+  # TODO(crbug.com/1024683): Enable for windows.
+  @unittest.skipIf(sys.platform == 'win32', 'not yet fixed on win')
+  def testRevertAndStatus(self):
     # Commit new change to repo to make repo_2's hash use a custom_var.
     cur_deps = self.FAKE_REPOS.git_hashes['repo_1'][-1][1]['DEPS']
     repo_2_hash = self.FAKE_REPOS.git_hashes['repo_2'][1][0][:7]
@@ -1629,14 +1580,14 @@ class GClientSmokeGITMutates(GClientSmokeBase):
       'origin': 'git/repo_1@3\n',
     })
 
-    config_template = (
+    config_template = ''.join([
-"""solutions = [{
+        'solutions = [{'
-  "name"        : "src",
+        '  "name"        : "src",'
-  "url"         : "%(git_base)srepo_1",
+        '  "url"         : %(git_base)r + "repo_1",'
-  "deps_file"   : "DEPS",
+        '  "deps_file"   : "DEPS",'
-  "managed"     : True,
+        '  "managed"     : True,'
-  "custom_vars" : %(custom_vars)s,
+        '  "custom_vars" : %(custom_vars)s,'
-}]""")
+        '}]'])
 
     self.gclient(['config', '--spec', config_template % {
       'git_base': self.git_base,
@@ -1693,8 +1644,6 @@ class GClientSmokeGITMutates(GClientSmokeBase):
     self.assertEqual(0, len(out))
 
   def testSyncNoHistory(self):
-    if not self.enabled:
-      return
     # Create an extra commit in repo_2 and point DEPS to its hash.
     cur_deps = self.FAKE_REPOS.git_hashes['repo_1'][-1][1]['DEPS']
     repo_2_hash_old = self.FAKE_REPOS.git_hashes['repo_2'][1][0][:7]
@@ -1709,13 +1658,13 @@ class GClientSmokeGITMutates(GClientSmokeBase):
       'origin': 'git/repo_1@4\n',
     })
 
-    config_template = (
+    config_template = ''.join([
-"""solutions = [{
+        'solutions = [{'
-"name"        : "src",
+        '  "name"        : "src",'
-"url"         : "%(git_base)srepo_1",
+        '  "url"         : %(git_base)r + "repo_1",'
-"deps_file"   : "DEPS",
+        '  "deps_file"   : "DEPS",'
-"managed"     : True,
+        '  "managed"     : True,'
-}]""")
+        '}]'])
 
     self.gclient(['config', '--spec', config_template % {
       'git_base': self.git_base
@@ -1742,18 +1691,17 @@ class SkiaDEPSTransitionSmokeTest(GClientSmokeBase):
   def setUp(self):
     super(SkiaDEPSTransitionSmokeTest, self).setUp()
     self.enabled = self.FAKE_REPOS.set_up_git()
-
-  def testSkiaDEPSChangeGit(self):
     if not self.enabled:
-      return
+      self.skipTest('git fake repos not available')
 
+  def testSkiaDEPSChangeGit(self):
     # Create an initial checkout:
     # - Single checkout at the root.
     # - Multiple checkouts in a shared subdirectory.
     self.gclient(['config', '--spec',
         'solutions=['
         '{"name": "src",'
-        ' "url": "' + self.git_base + 'repo_2",'
+        ' "url": ' + repr(self.git_base )+ '+ "repo_2",'
         '}]'])
 
     checkout_path = os.path.join(self.root_dir, 'src')
@@ -1828,6 +1776,8 @@ class BlinkDEPSTransitionSmokeTest(GClientSmokeBase):
   def setUp(self):
     super(BlinkDEPSTransitionSmokeTest, self).setUp()
     self.enabled = self.FAKE_REPOS.set_up_git()
+    if not self.enabled:
+      self.skipTest('git fake repos not available')
     self.checkout_path = os.path.join(self.root_dir, 'src')
     self.blink = os.path.join(self.checkout_path, 'third_party', 'WebKit')
     self.blink_git_url = self.FAKE_REPOS.git_base + 'repo_2'
@@ -1867,9 +1817,6 @@ class BlinkDEPSTransitionSmokeTest(GClientSmokeBase):
   def testBlinkDEPSChangeUsingGclient(self):
     """Checks that {src,blink} repos are consistent when syncing going back and
     forth using gclient sync src@revision."""
-    if not self.enabled:
-      return
-
     self.gclient(['config', '--spec',
         'solutions=['
         '{"name": "src",'
@@ -1893,9 +1840,6 @@ class BlinkDEPSTransitionSmokeTest(GClientSmokeBase):
   def testBlinkDEPSChangeUsingGit(self):
     """Like testBlinkDEPSChangeUsingGclient, but move the main project using
     directly git and not gclient sync."""
-    if not self.enabled:
-      return
-
     self.gclient(['config', '--spec',
         'solutions=['
         '{"name": "src",'
@@ -1926,9 +1870,6 @@ class BlinkDEPSTransitionSmokeTest(GClientSmokeBase):
   def testBlinkLocalBranchesArePreserved(self):
     """Checks that the state of local git branches are effectively preserved
     when going back and forth."""
-    if not self.enabled:
-      return
-
     self.gclient(['config', '--spec',
         'solutions=['
         '{"name": "src",'
@@ -1958,6 +1899,8 @@ class GClientSmokeCipd(GClientSmokeBase):
   def setUp(self):
     super(GClientSmokeCipd, self).setUp()
     self.enabled = self.FAKE_REPOS.set_up_git()
+    if not self.enabled:
+      self.skipTest('git fake repos not available')
     self.env['PATH'] = (os.path.join(ROOT_DIR, 'testing_support')
                         + os.pathsep + self.env['PATH'])