Merge ~nacc/git-ubuntu:test_fixtures-chdir into git-ubuntu:master

[Nish Aravamudan]

Make jenkins happy.

[Andreas Hasenack (ahasenack)]

Just a typo fix inline, +1

That being said, I was expecting to see some chdir calls being dropped, since the directory is changed in the fixture now. Or is this for tests yet to be written?

review: Approve

[Nish Aravamudan (nacc)]

On Tue, Apr 10, 2018 at 11:08 AM, Andreas Hasenack
<email address hidden> wrote:
> Review: Approve
>
> Just a typo fix inline, +1

Will fix.

> That being said, I was expecting to see some chdir calls being dropped, since the directory is changed in the fixture now. Or is this for tests yet to be written?

Nothing is actually testing the importer itself yet, which is what is
sensitive to the env like this. So yeah, for future tests.

>
> Diff comments:
>
>> diff --git a/gitubuntu/test_fixtures.py b/gitubuntu/test_fixtures.py
>> index de4cf7c..1312133 100644
>> --- a/gitubuntu/test_fixtures.py
>> +++ b/gitubuntu/test_fixtures.py
>> @@ -8,9 +9,28 @@ import gitubuntu.git_repository
>>
>> @pytest.fixture
>> def repo():
>> - """A empty GitUbuntuRepository repository fixture"""
>> + """A empty GitUbuntuRepository repository fixture
>
> s/A/An/
>
>> +
>> + The GitUbuntuRepository class methods (other than main entry point)
>> + assume they are running with the current work directory set to the
>> + Git repository's filesystem path. Additionally, pristine-tar and
>> + other commands require this to be the case, so chdir in the fixture
>> + itself.
>> + """
>> + try:
>> + oldcwd = os.getcwd()
>> + except OSError:
>> + oldcwd = None
>> +
>> with tempfile.TemporaryDirectory() as path:
>> - yield gitubuntu.git_repository.GitUbuntuRepository(path)
>> + os.chdir(path)
>> + try:
>> + yield gitubuntu.git_repository.GitUbuntuRepository(path)
>> + except:
>> + raise
>> + finally:
>> + if oldcwd:
>> + os.chdir(oldcwd)
>>
>>
>> @pytest.fixture
>
>
> --
> https://code.launchpad.net/~nacc/usd-importer/+git/usd-importer/+merge/342953
> Your team Ubuntu Server Dev import team is subscribed to branch usd-importer:master.
>
> --
> Usd-import-announce mailing list
> <email address hidden>
> Modify settings or unsubscribe at: https://lists.canonical.com/mailman/listinfo/usd-import-announce

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:e9a1cccd6cd9e1666e742e557aff93e23c48c2d9
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/390/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/390/rebuild

review: Approve (continuous-integration)

[Andreas Hasenack (ahasenack)]

Thanks for the explanation, +1 again

review: Approve

diff --git a/gitubuntu/test_fixtures.py b/gitubuntu/test_fixtures.py
index de4cf7c..06d24d6 100644
--- a/gitubuntu/test_fixtures.py
+++ b/gitubuntu/test_fixtures.py
@@ -1,3 +1,4 @@
+import os
 import tempfile
 
 import pygit2
@@ -8,13 +9,32 @@ import gitubuntu.git_repository
 
 @pytest.fixture
 def repo():
-    """A empty GitUbuntuRepository repository fixture"""
+    """An empty GitUbuntuRepository repository fixture
+
+    The GitUbuntuRepository class methods (other than main entry point)
+    assume they are running with the current work directory set to the
+    Git repository's filesystem path. Additionally, pristine-tar and
+    other commands require this to be the case, so chdir in the fixture
+    itself.
+    """
+    try:
+        oldcwd = os.getcwd()
+    except OSError:
+        oldcwd = None
+
     with tempfile.TemporaryDirectory() as path:
-        yield gitubuntu.git_repository.GitUbuntuRepository(path)
+        os.chdir(path)
+        try:
+            yield gitubuntu.git_repository.GitUbuntuRepository(path)
+        except:
+            raise
+        finally:
+            if oldcwd:
+                os.chdir(oldcwd)
 
 
 @pytest.fixture
 def pygit2_repo():
-    """A empty pygit2 repository fixture in a temporary directory"""
+    """An empty pygit2 repository fixture in a temporary directory"""
     with tempfile.TemporaryDirectory() as tmp_dir:
         yield pygit2.init_repository(tmp_dir)