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()`
This commit is contained in:
		 Zack Galbreath
					Zack Galbreath
				
			
				
					committed by
					
						 Todd Gamblin
						Todd Gamblin
					
				
			
			
				
	
			
			
			 Todd Gamblin
						Todd Gamblin
					
				
			
						parent
						
							b1a5764956
						
					
				
				
					commit
					f613437a44
				
			| @@ -26,7 +26,6 @@ | ||||
|  | ||||
| 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.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): | ||||
|   | ||||
| @@ -33,8 +33,9 @@ | ||||
| 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') | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
		Reference in New Issue
	
	Block a user