From e3148cd8078a1990ff36ac1fee67ed7ede5e07de Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Tue, 18 Aug 2020 17:47:01 -0600 Subject: Change erorr in case not-found Tool requested On the premise that the previous error - with a lengthy traceback and message "SConsEnvironmentError: No module named foo:" isn't very communicative - the error is changed to a UserError, which limits the traceback to just the SConstruct line, and the message is updated and now includes the tool dirs searched. This is assuming that putting an unknown tool (or a tool in in incorrectly specified path) into an SConscript is an error in the SConscript, thus UserError. The two affected unit tests are updated to expect UserError. Along the way, an existing technique from one of the unit tests is moved over to the other, allowing the elimination of a bare raise which was flagged by pylint (marked TODO in the file). The same change is then applied to a third unit test, unrelated but marked with the same TODO. Signed-off-by: Mats Wichmann --- CHANGES.txt | 2 ++ SCons/EnvironmentTests.py | 8 ++++---- SCons/Platform/PlatformTests.py | 7 +++---- SCons/Tool/ToolTests.py | 14 +++++++------- SCons/Tool/__init__.py | 9 ++++++--- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 6293590..4ab0aae 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -35,6 +35,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER OverrideEnvironment. Enable env.setdefault() method, add tests. - Raise an error if an option (not otherwise consumed) is used which looks like an abbreviation of one one added by AddOption. (#3653) + - Change error (to UserError) and error message in case a requested + Tool module is not found. From Joachim Kuebart: - Suppress missing SConscript deprecation warning if `must_exist=False` diff --git a/SCons/EnvironmentTests.py b/SCons/EnvironmentTests.py index 7a26a61..5a1ddd0 100644 --- a/SCons/EnvironmentTests.py +++ b/SCons/EnvironmentTests.py @@ -2456,16 +2456,16 @@ f5: \ exc_caught = None try: env.Tool('does_not_exist') - except SCons.Errors.SConsEnvironmentError: + except SCons.Errors.UserError: exc_caught = 1 - assert exc_caught, "did not catch expected SConsEnvironmentError" + assert exc_caught, "did not catch expected UserError" exc_caught = None try: env.Tool('$NONE') - except SCons.Errors.SConsEnvironmentError: + except SCons.Errors.UserError: exc_caught = 1 - assert exc_caught, "did not catch expected SConsEnvironmentError" + assert exc_caught, "did not catch expected UserError" # Use a non-existent toolpath directory just to make sure we # can call Tool() with the keyword argument. diff --git a/SCons/Platform/PlatformTests.py b/SCons/Platform/PlatformTests.py index e065c3c..4186119 100644 --- a/SCons/Platform/PlatformTests.py +++ b/SCons/Platform/PlatformTests.py @@ -107,14 +107,13 @@ class PlatformTestCase(unittest.TestCase): p(env) assert env['PROGSUFFIX'] == '.exe', env assert env['LIBSUFFIX'] == '.lib', env - assert str + exc_caught = None try: p = SCons.Platform.Platform('_does_not_exist_') except SCons.Errors.UserError: - pass - else: # TODO pylint E0704: bare raise not inside except - raise + exc_caught = 1 + assert exc_caught, "did not catch expected UserError" env = Environment() SCons.Platform.Platform()(env) diff --git a/SCons/Tool/ToolTests.py b/SCons/Tool/ToolTests.py index 75a9454..33eb14c 100644 --- a/SCons/Tool/ToolTests.py +++ b/SCons/Tool/ToolTests.py @@ -84,19 +84,19 @@ class ToolTestCase(unittest.TestCase): assert env['INCPREFIX'] == '-I', env['INCPREFIX'] assert env['TOOLS'] == ['g++'], env['TOOLS'] + exc_caught = None try: SCons.Tool.Tool() except TypeError: - pass - else: # TODO pylint E0704: bare raise not inside except - raise + exc_caught = 1 + assert exc_caught, "did not catch expected UserError" + exc_caught = None try: p = SCons.Tool.Tool('_does_not_exist_') - except SCons.Errors.SConsEnvironmentError: - pass - else: # TODO pylint E0704: bare raise not inside except - raise + except SCons.Errors.UserError: + exc_caught = 1 + assert exc_caught, "did not catch expected UserError" def test_pathfind(self): diff --git a/SCons/Tool/__init__.py b/SCons/Tool/__init__.py index 24fb9d2..0c7afb8 100644 --- a/SCons/Tool/__init__.py +++ b/SCons/Tool/__init__.py @@ -184,13 +184,16 @@ class Tool: if debug: sys.stderr.write("Spec Found? .%s :%s\n" % (self.name, spec)) if spec is None: - error_string = "No module named %s" % self.name - raise SCons.Errors.SConsEnvironmentError(error_string) + sconstools = os.path.normpath(sys.modules['SCons.Tool'].__path__[0]) + if self.toolpath: + sconstools = ", ".join(self.toolpath) + ", " + sconstools + error_string = "No tool module '%s' found in %s" % (self.name, sconstools) + raise SCons.Errors.UserError(error_string) module = importlib.util.module_from_spec(spec) if module is None: if debug: print("MODULE IS NONE:%s" % self.name) - error_string = "No module named %s" % self.name + error_string = "Tool module '%s' failed import" % self.name raise SCons.Errors.SConsEnvironmentError(error_string) # Don't reload a tool we already loaded. -- cgit v0.12 From c907fbc37f44cdb933b4722fbb614337f470b028 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 9 Sep 2020 07:21:24 -0600 Subject: Add check for new not-found Tool errmsg Signed-off-by: Mats Wichmann --- SCons/Tool/ToolTests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SCons/Tool/ToolTests.py b/SCons/Tool/ToolTests.py index 33eb14c..b7c42b7 100644 --- a/SCons/Tool/ToolTests.py +++ b/SCons/Tool/ToolTests.py @@ -94,8 +94,10 @@ class ToolTestCase(unittest.TestCase): exc_caught = None try: p = SCons.Tool.Tool('_does_not_exist_') - except SCons.Errors.UserError: + except SCons.Errors.UserError as e: exc_caught = 1 + # Old msg was Python-style "No tool named", check for new msg: + assert "No tool module" in str(e), e assert exc_caught, "did not catch expected UserError" -- cgit v0.12 From 9b55e054df7408831445af12d7571824a718a5c7 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Thu, 10 Sep 2020 08:16:26 -0600 Subject: Update CHANGES.txt msg for Tool not found [ci skip] Signed-off-by: Mats Wichmann --- CHANGES.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 4ab0aae..841a279 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -35,8 +35,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER OverrideEnvironment. Enable env.setdefault() method, add tests. - Raise an error if an option (not otherwise consumed) is used which looks like an abbreviation of one one added by AddOption. (#3653) - - Change error (to UserError) and error message in case a requested - Tool module is not found. + - Tool module not found is now a UserError to more clearly indicate this is + probably an SConscript problem, and to make the traceback more relevant. From Joachim Kuebart: - Suppress missing SConscript deprecation warning if `must_exist=False` -- cgit v0.12