Merge pull request #35 from swansonk14/windows-tests

swansonk14 · web-flow · commit 76f8bc34a3bd · 2021-01-10T15:21:59.000Z
Successful attempt to fix windows tests
diff --git a/.github/workflows/tap-code-coverage.yml b/.github/workflows/tap-code-coverage.yml
@@ -2,8 +2,6 @@ name: code-coverage
 on:
   push:
     branches: [ master ]
-  pull_request:
-    branches: [ master ]
 jobs:
   run:
     runs-on: ubuntu-latest
diff --git a/.github/workflows/tap-tests.yml b/.github/workflows/tap-tests.yml
@@ -15,7 +15,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        os: [ubuntu-latest, macos-latest]
+        os: [ubuntu-latest, macos-latest, windows-latest]
         python-version: [3.6, 3.7, 3.8, 3.9]
 
     steps:
diff --git a/README.md b/README.md
@@ -7,8 +7,8 @@
 [![PyPI - Python Version](https://img.shields.io/pypi/pyversions/typed-argument-parser)](https://badge.fury.io/py/typed-argument-parser)
 [![PyPI version](https://badge.fury.io/py/typed-argument-parser.svg)](https://badge.fury.io/py/typed-argument-parser)
 [![Build Status](https://github.com/swansonk14/typed-argument-parser/workflows/tests/badge.svg)](https://github.com/swansonk14/typed-argument-parser)
-[![license](https://img.shields.io/github/license/swansonk14/typed-argument-parser.svg)](https://github.com/swansonk14/typed-argument-parser/blob/master/LICENSE.txt)
 [![codecov](https://codecov.io/gh/swansonk14/typed-argument-parser/branch/master/graph/badge.svg)](https://codecov.io/gh/swansonk14/typed-argument-parser)
+[![license](https://img.shields.io/github/license/swansonk14/typed-argument-parser.svg)](https://github.com/swansonk14/typed-argument-parser/blob/master/LICENSE.txt)
 
 Tap is a typed modernization of Python's [argparse](https://docs.python.org/3/library/argparse.html) library.
 
diff --git a/tap/utils.py b/tap/utils.py
@@ -90,7 +90,7 @@ def get_git_url(commit_hash: bool = True) -> str:
 
     if commit_hash:
         # Add tree and hash of current commit
-        url = os.path.join(url, 'tree', get_git_hash())
+        url = f'{url}/tree/{get_git_hash()}'
 
     return url
 
diff --git a/tests/test_integration.py b/tests/test_integration.py
@@ -1,6 +1,7 @@
 from copy import deepcopy
+import os
 import sys
-from tempfile import NamedTemporaryFile
+from tempfile import TemporaryDirectory
 from typing import Any, List, Optional, Set, Tuple
 from typing_extensions import Literal
 import unittest
@@ -1023,10 +1024,11 @@ class SimpleSaveLoadTap(Tap):
 
         args = SimpleSaveLoadTap().parse_args(['--a', 'hi'])
 
-        with NamedTemporaryFile() as f:
-            args.save(f.name)
+        with TemporaryDirectory() as temp_dir:
+            fname = os.path.join(temp_dir, 'args.json')
+            args.save(fname)
             new_args = SimpleSaveLoadTap()
-            new_args.load(f.name)
+            new_args.load(fname)
 
         output = {'a': 'hi', 'b': 1, 'c': True}
         self.assertEqual(output, new_args.as_dict())
@@ -1037,9 +1039,10 @@ class SaveLoadReturnTap(Tap):
 
         args = SaveLoadReturnTap().parse_args(['--a', 'hi'])
 
-        with NamedTemporaryFile() as f:
-            args.save(f.name)
-            new_args = SaveLoadReturnTap().load(f.name)
+        with TemporaryDirectory() as temp_dir:
+            fname = os.path.join(temp_dir, 'args.json')
+            args.save(fname)
+            new_args = SaveLoadReturnTap().load(fname)
 
         output = {'a': 'hi'}
         self.assertEqual(output, new_args.as_dict())
@@ -1059,10 +1062,11 @@ def configure(self) -> None:
 
         args = ComplexSaveLoadTap().parse_args(['--a', 'hi', '--d', 'a', 'b', '--e', '7', '--g', 'tapper'])
 
-        with NamedTemporaryFile() as f:
-            args.save(f.name)
+        with TemporaryDirectory() as temp_dir:
+            fname = os.path.join(temp_dir, 'args.json')
+            args.save(fname)
             new_args = ComplexSaveLoadTap()
-            new_args.load(f.name)
+            new_args.load(fname)
 
         output = {'e': 7, 'f': {1}, 'd': ('a', 'b'), 'c': True, 'a': 'hi', 'b': 1, 'g': Person('tapper')}
         self.assertEqual(output, new_args.as_dict())
@@ -1094,14 +1098,15 @@ def process_args(self) -> None:
 
         args = PropertySaveLoadTap().parse_args([])
 
-        with NamedTemporaryFile() as f:
-            args.save(f.name)
+        with TemporaryDirectory() as temp_dir:
+            fname = os.path.join(temp_dir, 'args.json')
+            args.save(fname)
             new_args = PropertySaveLoadTap()
 
             with self.assertRaises(AttributeError):
-                new_args.load(f.name)  # because trying to set unsettable prop1
+                new_args.load(fname)  # because trying to set unsettable prop1
 
-            new_args.load(f.name, skip_unsettable=True)
+            new_args.load(fname, skip_unsettable=True)
 
         output = {'a': 'hello', 'prop1': 1, 'prop2': 'bye'}
         self.assertEqual(output, new_args.as_dict())
diff --git a/tests/test_load_config_files.py b/tests/test_load_config_files.py
@@ -1,5 +1,6 @@
+import os
 import sys
-from tempfile import NamedTemporaryFile
+from tempfile import TemporaryDirectory
 import unittest
 from unittest import TestCase
 
@@ -26,10 +27,13 @@ class SimpleTap(Tap):
             a: int
             b: str = 'b'
 
-        with NamedTemporaryFile() as f:
-            f.write(b'--a 1')
-            f.flush()
-            args = SimpleTap(config_files=[f.name]).parse_args([])
+        with TemporaryDirectory() as temp_dir:
+            fname = os.path.join(temp_dir, 'config.txt')
+
+            with open(fname, 'w') as f:
+                f.write('--a 1')
+
+            args = SimpleTap(config_files=[fname]).parse_args([])
 
         self.assertEqual(args.a, 1)
         self.assertEqual(args.b, 'b')
@@ -39,10 +43,13 @@ class SimpleOverwritingTap(Tap):
             a: int
             b: str = 'b'
 
-        with NamedTemporaryFile() as f:
-            f.write(b'--a 1 --b two')
-            f.flush()
-            args = SimpleOverwritingTap(config_files=[f.name]).parse_args('--a 2'.split())
+        with TemporaryDirectory() as temp_dir:
+            fname = os.path.join(temp_dir, 'config.txt')
+
+            with open(fname, 'w') as f:
+                f.write('--a 1 --b two')
+
+            args = SimpleOverwritingTap(config_files=[fname]).parse_args('--a 2'.split())
 
         self.assertEqual(args.a, 2)
         self.assertEqual(args.b, 'two')
@@ -52,10 +59,13 @@ class KnownOnlyTap(Tap):
             a: int
             b: str = 'b'
 
-        with NamedTemporaryFile() as f:
-            f.write(b'--a 1 --c seeNothing')
-            f.flush()
-            args = KnownOnlyTap(config_files=[f.name]).parse_args([], known_only=True)
+        with TemporaryDirectory() as temp_dir:
+            fname = os.path.join(temp_dir, 'config.txt')
+
+            with open(fname, 'w') as f:
+                f.write('--a 1 --c seeNothing')
+
+            args = KnownOnlyTap(config_files=[fname]).parse_args([], known_only=True)
 
         self.assertEqual(args.a, 1)
         self.assertEqual(args.b, 'b')
@@ -66,23 +76,28 @@ class KnownOnlyTap(Tap):
             a: int
             b: str = 'b'
 
-        with NamedTemporaryFile() as f, self.assertRaises(SystemExit):
+        with TemporaryDirectory() as temp_dir, self.assertRaises(SystemExit):
             sys.stderr = self.dev_null
-            f.write(b'--b fore')
-            f.flush()
-            KnownOnlyTap(config_files=[f.name]).parse_args([])
+            fname = os.path.join(temp_dir, 'config.txt')
+
+            with open(fname, 'w') as f:
+                f.write('--b fore')
+
+            KnownOnlyTap(config_files=[fname]).parse_args([])
 
     def test_multiple_configs(self) -> None:
         class MultipleTap(Tap):
             a: int
             b: str = 'b'
 
-        with NamedTemporaryFile() as f1, NamedTemporaryFile() as f2:
-            f1.write(b'--b two')
-            f1.flush()
-            f2.write(b'--a 1')
-            f2.flush()
-            args = MultipleTap(config_files=[f1.name, f2.name]).parse_args([])
+        with TemporaryDirectory() as temp_dir:
+            fname1, fname2 = os.path.join(temp_dir, 'config1.txt'), os.path.join(temp_dir, 'config2.txt')
+
+            with open(fname1, 'w') as f1, open(fname2, 'w') as f2:
+                f1.write('--b two')
+                f2.write('--a 1')
+
+            args = MultipleTap(config_files=[fname1, fname2]).parse_args([])
 
         self.assertEqual(args.a, 1)
         self.assertEqual(args.b, 'two')
@@ -93,12 +108,14 @@ class MultipleOverwritingTap(Tap):
             b: str = 'b'
             c: str = 'c'
 
-        with NamedTemporaryFile() as f1, NamedTemporaryFile() as f2:
-            f1.write(b'--a 1 --b two')
-            f1.flush()
-            f2.write(b'--a 2 --c see')
-            f2.flush()
-            args = MultipleOverwritingTap(config_files=[f1.name, f2.name]).parse_args('--b four'.split())
+        with TemporaryDirectory() as temp_dir:
+            fname1, fname2 = os.path.join(temp_dir, 'config1.txt'), os.path.join(temp_dir, 'config2.txt')
+
+            with open(fname1, 'w') as f1, open(fname2, 'w') as f2:
+                f1.write('--a 1 --b two')
+                f2.write('--a 2 --c see')
+
+            args = MultipleOverwritingTap(config_files=[fname1, fname2]).parse_args('--b four'.split())
 
         self.assertEqual(args.a, 2)
         self.assertEqual(args.b, 'four')
@@ -109,11 +126,14 @@ class JunkConfigTap(Tap):
             a: int
             b: str = 'b'
 
-        with NamedTemporaryFile() as f1, self.assertRaises(SystemExit):
+        with TemporaryDirectory() as temp_dir, self.assertRaises(SystemExit):
             sys.stderr = self.dev_null
-            f1.write(b'is not a file that can reasonably be parsed')
-            f1.flush()
-            JunkConfigTap(config_files=[f1.name]).parse_args()
+            fname = os.path.join(temp_dir, 'config.txt')
+
+            with open(fname, 'w') as f:
+                f.write('is not a file that can reasonably be parsed')
+
+            JunkConfigTap(config_files=[fname]).parse_args()
 
 
 if __name__ == '__main__':
diff --git a/tests/test_utils.py b/tests/test_utils.py
@@ -1,6 +1,7 @@
 from collections import OrderedDict
 import json
 import os
+import platform
 import subprocess
 from tempfile import TemporaryDirectory
 from typing import Any, Callable, List, Dict, Set, Tuple, Union
@@ -28,6 +29,7 @@
 class GitTests(TestCase):
     def setUp(self) -> None:
         self.temp_dir = TemporaryDirectory()
+        self.prev_dir = os.getcwd()
         os.chdir(self.temp_dir.name)
         subprocess.check_output(['git', 'init'])
         self.url = 'https://github.com/test_account/test_repo'
@@ -37,6 +39,13 @@ def setUp(self) -> None:
         subprocess.check_output(['git', 'commit', '-m', 'Initial commit'])
 
     def tearDown(self) -> None:
+        os.chdir(self.prev_dir)
+
+        # Add permissions to temporary directory to enable cleanup in Windows
+        for root, dirs, files in os.walk(self.temp_dir.name):
+            for name in dirs + files:
+                os.chmod(os.path.join(root, name), 0o777)
+
         self.temp_dir.cleanup()
 
     def test_has_git_true(self) -> None:
@@ -46,14 +55,25 @@ def test_has_git_false(self) -> None:
         with TemporaryDirectory() as temp_dir_no_git:
             os.chdir(temp_dir_no_git)
             self.assertFalse(has_git())
-        os.chdir(self.temp_dir.name)
+            os.chdir(self.temp_dir.name)
 
+    # TODO: fix this test on Windows
+    @unittest.skipIf(platform.system() == 'Windows', 'Inconsistent user path on Windows in GitHub Actions')
     def test_get_git_root(self) -> None:
-        self.assertTrue(get_git_root() in f'/private{self.temp_dir.name}')
+        # Ideally should be self.temp_dir.name == get_git_root() but the OS may add a prefix like /private
+        self.assertTrue(get_git_root().endswith(self.temp_dir.name))
 
+    # TODO: fix this test on Windows
+    @unittest.skipIf(platform.system() == 'Windows', 'Inconsistent user path on Windows in GitHub Actions')
     def test_get_git_root_subdir(self) -> None:
-        os.makedirs(os.path.join(self.temp_dir.name, 'subdir'))
-        self.assertTrue(get_git_root() in f'/private{self.temp_dir.name}')
+        subdir = os.path.join(self.temp_dir.name, 'subdir')
+        os.makedirs(subdir)
+        os.chdir(subdir)
+
+        # Ideally should be self.temp_dir.name == get_git_root() but the OS may add a prefix like /private
+        self.assertTrue(get_git_root().endswith(self.temp_dir.name))
+
+        os.chdir(self.temp_dir.name)
 
     def test_get_git_url_https(self) -> None:
         self.assertEqual(get_git_url(commit_hash=False), self.url)
