Do not call sys.exit() in except block (#7659)

When an invalid spec is encountered by `parse_specs()` we now raise a
`SpackError` instead of calling `sys.exit()`

3 files changed, 31 insertions, 12 deletions

diff --git a/lib/spack/spack/cmd/__init__.py b/lib/spack/spack/cmd/__init__.py
index 9ac28da198..f0e62fbc56 100644
--- a/lib/spack/spack/cmd/__init__.py
+++ b/lib/spack/spack/cmd/__init__.py
@@ -26,7 +26,6 @@ from __future__ import print_function
 
 import os
 import re
-import sys
 
 import llnl.util.tty as tty
 from llnl.util.lang import attr_setdefault, index_by
@@ -38,6 +37,7 @@ import spack
 import spack.config
 import spack.spec
 import spack.store
+from spack.error import SpackError
 
 #
 # Settings for commands that modify configuration
@@ -141,18 +141,18 @@ def parse_specs(args, **kwargs):
 
         return specs
 
-    except spack.parse.ParseError as e:
-        tty.error(e.message, e.string, e.pos * " " + "^")
-        sys.exit(1)
+    except spack.spec.SpecParseError as e:
+        msg = e.message + "\n" + str(e.string) + "\n"
+        msg += (e.pos + 2) * " " + "^"
+        raise SpackError(msg)
 
     except spack.spec.SpecError as e:
 
-        msgs = [e.message]
+        msg = e.message
         if e.long_message:
-            msgs.append(e.long_message)
+            msg += e.long_message
 
-        tty.error(*msgs)
-        sys.exit(1)
+        raise SpackError(msg)
 
 
 def elide_list(line_list, max_num=10):
diff --git a/lib/spack/spack/test/cmd/install.py b/lib/spack/spack/test/cmd/install.py
index a6a93d2fea..91d2bbc7ed 100644
--- a/lib/spack/spack/test/cmd/install.py
+++ b/lib/spack/spack/test/cmd/install.py
@@ -33,8 +33,9 @@ import llnl.util.filesystem as fs
 import spack
 import spack.cmd.install
 import spack.package
+from spack.error import SpackError
 from spack.spec import Spec
-from spack.main import SpackCommand, SpackCommandError
+from spack.main import SpackCommand
 
 install = SpackCommand('install')
 
@@ -238,11 +239,18 @@ def test_install_overwrite(
     'builtin_mock', 'mock_archive', 'mock_fetch', 'config', 'install_mockery',
 )
 def test_install_conflicts(conflict_spec):
-    # Make sure that spec with conflicts exit with 1
-    with pytest.raises(SpackCommandError):
+    # Make sure that spec with conflicts raises a SpackError
+    with pytest.raises(SpackError):
         install(conflict_spec)
 
-    assert install.returncode == 1
+
+@pytest.mark.usefixtures(
+    'builtin_mock', 'mock_archive', 'mock_fetch', 'config', 'install_mockery',
+)
+def test_install_invalid_spec(invalid_spec):
+    # Make sure that invalid specs raise a SpackError
+    with pytest.raises(SpackError, match='Unexpected token'):
+        install(invalid_spec)
 
 
 @pytest.mark.usefixtures('noop_install', 'config')
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index 0e98a1cf00..3cee46cbf2 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -709,3 +709,14 @@ def conflict_spec(request):
     directive in the "conflict" package.
     """
     return request.param
+
+
+@pytest.fixture(
+    params=[
+        'conflict%~'
+    ]
+)
+def invalid_spec(request):
+    """Specs that do not parse cleanly due to invalid formatting.
+    """
+    return request.param