diff mbox series

[1/5] tests: git-lfs: Restore _find_git_lfs

Message ID 20230217170009.58707-1-paulo@myneves.com
State New
Headers show
Series [1/5] tests: git-lfs: Restore _find_git_lfs | expand

Commit Message

Paulo Neves Feb. 17, 2023, 5 p.m. UTC
Not restoring the mocked _find_git_lfs leads to other tests
failing.

Signed-off-by: Paulo Neves <paulo@myneves.com>
---
 lib/bb/tests/fetch.py | 40 ++++++++++++++++++++++++----------------
 1 file changed, 24 insertions(+), 16 deletions(-)

--
2.34.1

Comments

Richard Purdie Feb. 22, 2023, 12:01 p.m. UTC | #1
On Fri, 2023-02-17 at 17:00 +0000, Paulo Neves wrote:
> Not restoring the mocked _find_git_lfs leads to other tests
> failing.
> 
> Signed-off-by: Paulo Neves <paulo@myneves.com>
> ---
>  lib/bb/tests/fetch.py | 40 ++++++++++++++++++++++++----------------
>  1 file changed, 24 insertions(+), 16 deletions(-)

Just for future reference, the shortlog entries for these patches needs
tweaking. For example in this case I'd have written:

tests/fetch: git-lfs restore _find_git_lfs

since that makes it clear it is the fetch test being changed.

In other changes you used both "git" and "git.py", I'd suggest just
using "git" and I'd probably use "fetch/git" to make it clear it was
the git fetcher.

I'll fix up these in the patch queue this time so no need to resend.

Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/tests/fetch.py b/lib/bb/tests/fetch.py
index f3890321d..6d16cf59a 100644
--- a/lib/bb/tests/fetch.py
+++ b/lib/bb/tests/fetch.py
@@ -2250,12 +2250,16 @@  class GitLfsTest(FetcherTest):
             shutil.rmtree(self.gitdir, ignore_errors=True)
             fetcher.unpack(self.d.getVar('WORKDIR'))

-        # If git-lfs cannot be found, the unpack should throw an error
-        with self.assertRaises(bb.fetch2.FetchError):
-            fetcher.download()
-            ud.method._find_git_lfs = lambda d: False
-            shutil.rmtree(self.gitdir, ignore_errors=True)
-            fetcher.unpack(self.d.getVar('WORKDIR'))
+        old_find_git_lfs = ud.method._find_git_lfs
+        try:
+            # If git-lfs cannot be found, the unpack should throw an error
+            with self.assertRaises(bb.fetch2.FetchError):
+                fetcher.download()
+                ud.method._find_git_lfs = lambda d: False
+                shutil.rmtree(self.gitdir, ignore_errors=True)
+                fetcher.unpack(self.d.getVar('WORKDIR'))
+        finally:
+            ud.method._find_git_lfs = old_find_git_lfs

     def test_lfs_disabled(self):
         import shutil
@@ -2270,17 +2274,21 @@  class GitLfsTest(FetcherTest):
         fetcher, ud = self.fetch()
         self.assertIsNotNone(ud.method._find_git_lfs)

-        # If git-lfs can be found, the unpack should be successful. A
-        # live copy of git-lfs is not required for this case, so
-        # unconditionally forge its presence.
-        ud.method._find_git_lfs = lambda d: True
-        shutil.rmtree(self.gitdir, ignore_errors=True)
-        fetcher.unpack(self.d.getVar('WORKDIR'))
+        old_find_git_lfs = ud.method._find_git_lfs
+        try:
+            # If git-lfs can be found, the unpack should be successful. A
+            # live copy of git-lfs is not required for this case, so
+            # unconditionally forge its presence.
+            ud.method._find_git_lfs = lambda d: True
+            shutil.rmtree(self.gitdir, ignore_errors=True)
+            fetcher.unpack(self.d.getVar('WORKDIR'))
+            # If git-lfs cannot be found, the unpack should be successful

-        # If git-lfs cannot be found, the unpack should be successful
-        ud.method._find_git_lfs = lambda d: False
-        shutil.rmtree(self.gitdir, ignore_errors=True)
-        fetcher.unpack(self.d.getVar('WORKDIR'))
+            ud.method._find_git_lfs = lambda d: False
+            shutil.rmtree(self.gitdir, ignore_errors=True)
+            fetcher.unpack(self.d.getVar('WORKDIR'))
+        finally:
+            ud.method._find_git_lfs = old_find_git_lfs

 class GitURLWithSpacesTest(FetcherTest):
     test_git_urls = {