tests: add extra coverage for fetch strategy tests

4 files changed, 142 insertions, 8 deletions

diff --git a/lib/spack/spack/test/git_fetch.py b/lib/spack/spack/test/git_fetch.py
index cccdb7b829..e69a56596f 100644
--- a/lib/spack/spack/test/git_fetch.py
+++ b/lib/spack/spack/test/git_fetch.py
@@ -4,14 +4,16 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 import os
+import shutil
 
 import pytest
 
-from llnl.util.filesystem import working_dir, touch
+from llnl.util.filesystem import working_dir, touch, mkdirp
 
 import spack.repo
 import spack.config
 from spack.spec import Spec
+from spack.stage import Stage
 from spack.version import ver
 from spack.fetch_strategy import GitFetchStrategy
 from spack.util.executable import which
@@ -21,8 +23,11 @@ pytestmark = pytest.mark.skipif(
     not which('git'), reason='requires git to be installed')
 
 
+_mock_transport_error = 'Mock HTTP transport error'
+
+
 @pytest.fixture(params=[None, '1.8.5.2', '1.8.5.1', '1.7.10', '1.7.0'])
-def git_version(request):
+def git_version(request, monkeypatch):
     """Tests GitFetchStrategy behavior for different git versions.
 
     GitFetchStrategy tries to optimize using features of newer git
@@ -40,13 +45,38 @@ def git_version(request):
         if test_git_version > real_git_version:
             pytest.skip("Can't test clone logic for newer version of git.")
 
-        # patch the fetch strategy to think it's using a lower git version.
+        # Patch the fetch strategy to think it's using a lower git version.
         # we use this to test what we'd need to do with older git versions
         # using a newer git installation.
-        git_version_method = GitFetchStrategy.git_version
-        GitFetchStrategy.git_version = test_git_version
+        monkeypatch.setattr(GitFetchStrategy, 'git_version', ver('1.7.1'))
         yield
-        GitFetchStrategy.git_version = git_version_method
+
+
+@pytest.fixture
+def mock_bad_git(monkeypatch):
+    """
+    Test GitFetchStrategy behavior with a bad git command for git >= 1.7.1
+    to trigger a SpackError.
+    """
+    def bad_git(*args, **kwargs):
+        """Raise a SpackError with the transport message."""
+        raise spack.error.SpackError(_mock_transport_error)
+
+    # Patch the fetch strategy to think it's using a git version that
+    # will error out when git is called.
+    monkeypatch.setattr(GitFetchStrategy, 'git', bad_git)
+    monkeypatch.setattr(GitFetchStrategy, 'git_version', ver('1.7.1'))
+    yield
+
+
+def test_bad_git(tmpdir, mock_bad_git):
+    """Trigger a SpackError when attempt a fetch with a bad git."""
+    testpath = str(tmpdir)
+
+    with pytest.raises(spack.error.SpackError):
+        fetcher = GitFetchStrategy(git='file:///not-a-real-git-repo')
+        with Stage(fetcher, path=testpath):
+            fetcher.fetch()
 
 
 @pytest.mark.parametrize("type_of_test", ['master', 'branch', 'tag', 'commit'])
@@ -101,3 +131,41 @@ def test_fetch(type_of_test,
             assert os.path.isfile(file_path)
 
             assert h('HEAD') == h(t.revision)
+
+
+@pytest.mark.parametrize("type_of_test", ['branch', 'commit'])
+def test_debug_fetch(type_of_test, mock_git_repository, config):
+    """Fetch the repo with debug enabled."""
+    # Retrieve the right test parameters
+    t = mock_git_repository.checks[type_of_test]
+
+    # Construct the package under test
+    spec = Spec('git-test')
+    spec.concretize()
+    pkg = spack.repo.get(spec)
+    pkg.versions[ver('git')] = t.args
+
+    # Fetch then ensure source path exists
+    with pkg.stage:
+        with spack.config.override('config:debug', True):
+            pkg.do_fetch()
+            assert os.path.isdir(pkg.stage.source_path)
+
+
+def test_git_extra_fetch(tmpdir):
+    """Ensure a fetch after 'expanding' is effectively a no-op."""
+    testpath = str(tmpdir)
+
+    fetcher = GitFetchStrategy(git='file:///not-a-real-git-repo')
+    with Stage(fetcher, path=testpath) as stage:
+        mkdirp(stage.source_path)
+        fetcher.fetch()   # Use fetcher to fetch for code coverage
+        shutil.rmtree(stage.source_path)
+
+
+def test_needs_stage():
+    """Trigger a NoStageError when attempt a fetch without a stage."""
+    with pytest.raises(spack.fetch_strategy.NoStageError,
+                       matches=_mock_transport_error):
+        fetcher = GitFetchStrategy(git='file:///not-a-real-git-repo')
+        fetcher.fetch()
diff --git a/lib/spack/spack/test/hg_fetch.py b/lib/spack/spack/test/hg_fetch.py
index 779dfd5a4b..109e3d9d59 100644
--- a/lib/spack/spack/test/hg_fetch.py
+++ b/lib/spack/spack/test/hg_fetch.py
@@ -7,12 +7,14 @@ import os
 
 import pytest
 
-from llnl.util.filesystem import working_dir, touch
+from llnl.util.filesystem import working_dir, touch, mkdirp
 
 import spack.repo
 import spack.config
 from spack.spec import Spec
+from spack.stage import Stage
 from spack.version import ver
+from spack.fetch_strategy import HgFetchStrategy
 from spack.util.executable import which
 
 
@@ -73,3 +75,14 @@ def test_fetch(
             assert os.path.isfile(file_path)
 
             assert h() == t.revision
+
+
+def test_hg_extra_fetch(tmpdir):
+    """Ensure a fetch after expanding is effectively a no-op."""
+    testpath = str(tmpdir)
+
+    fetcher = HgFetchStrategy(hg='file:///not-a-real-hg-repo')
+    with Stage(fetcher, path=testpath) as stage:
+        source_path = stage.source_path
+        mkdirp(source_path)
+        fetcher.fetch()
diff --git a/lib/spack/spack/test/svn_fetch.py b/lib/spack/spack/test/svn_fetch.py
index 1542615326..a915a82874 100644
--- a/lib/spack/spack/test/svn_fetch.py
+++ b/lib/spack/spack/test/svn_fetch.py
@@ -7,12 +7,14 @@ import os
 
 import pytest
 
-from llnl.util.filesystem import touch, working_dir
+from llnl.util.filesystem import touch, working_dir, mkdirp
 
 import spack.repo
 import spack.config
 from spack.spec import Spec
+from spack.stage import Stage
 from spack.version import ver
+from spack.fetch_strategy import SvnFetchStrategy
 from spack.util.executable import which
 
 
@@ -73,3 +75,19 @@ def test_fetch(
             assert os.path.isfile(file_path)
 
             assert h() == t.revision
+
+
+def test_svn_extra_fetch(tmpdir):
+    """Ensure a fetch after downloading is effectively a no-op."""
+    testpath = str(tmpdir)
+
+    fetcher = SvnFetchStrategy(svn='file:///not-a-real-svn-repo')
+    assert fetcher is not None
+
+    with Stage(fetcher, path=testpath) as stage:
+        assert stage is not None
+
+        source_path = stage.source_path
+        mkdirp(source_path)
+
+        fetcher.fetch()
diff --git a/lib/spack/spack/test/url_fetch.py b/lib/spack/spack/test/url_fetch.py
index 4b794eb1c2..cc9560350a 100644
--- a/lib/spack/spack/test/url_fetch.py
+++ b/lib/spack/spack/test/url_fetch.py
@@ -10,8 +10,10 @@ from llnl.util.filesystem import working_dir, is_exe
 
 import spack.repo
 import spack.config
+from spack.fetch_strategy import FailedDownloadError
 from spack.fetch_strategy import from_list_url, URLFetchStrategy
 from spack.spec import Spec
+from spack.stage import Stage
 from spack.version import ver
 import spack.util.crypto as crypto
 
@@ -21,6 +23,27 @@ def checksum_type(request):
     return request.param
 
 
+def test_urlfetchstrategy_sans_url():
+    """Ensure constructor with no URL fails."""
+    with pytest.raises(ValueError):
+        with URLFetchStrategy(None):
+            pass
+
+
+def test_urlfetchstrategy_bad_url(tmpdir):
+    """Ensure fetch with bad URL fails as expected."""
+    testpath = str(tmpdir)
+
+    with pytest.raises(FailedDownloadError):
+        fetcher = URLFetchStrategy(url='file:///does-not-exist')
+        assert fetcher is not None
+
+        with Stage(fetcher, path=testpath) as stage:
+            assert stage is not None
+            assert fetcher.archive_file is None
+            fetcher.fetch()
+
+
 @pytest.mark.parametrize('secure', [True, False])
 def test_fetch(
         mock_archive,
@@ -135,3 +158,15 @@ def test_hash_detection(checksum_type):
 def test_unknown_hash(checksum_type):
     with pytest.raises(ValueError):
         crypto.Checker('a')
+
+
+def test_url_extra_fetch(tmpdir, mock_archive):
+    """Ensure a fetch after downloading is effectively a no-op."""
+    testpath = str(tmpdir)
+
+    fetcher = URLFetchStrategy(mock_archive.url)
+    with Stage(fetcher, path=testpath) as stage:
+        assert fetcher.archive_file is None
+        stage.fetch()
+        assert fetcher.archive_file is not None
+        fetcher.fetch()