From e8a85f86a0d20f0c0a65ead4ccc2a37486aca899 Mon Sep 17 00:00:00 2001 From: Bryan Oakley Date: Tue, 24 Nov 2020 18:30:58 -0600 Subject: [PATCH 1/2] Add repo root to sys.path in robotframework task --- .../tasks/robotframework/robotframework.py | 12 +++++++++ .../tests/test_robotframework.py | 27 ++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/cumulusci/tasks/robotframework/robotframework.py b/cumulusci/tasks/robotframework/robotframework.py index 7991a9318c..7035705afd 100644 --- a/cumulusci/tasks/robotframework/robotframework.py +++ b/cumulusci/tasks/robotframework/robotframework.py @@ -86,6 +86,7 @@ def _init_options(self, kwargs): ) listeners = self.options["options"].setdefault("listener", []) + if process_bool_arg(self.options.get("verbose") or False): listeners.append(KeywordLogger()) @@ -170,6 +171,17 @@ def _run_task(self): for path in source_paths.values(): pythonpathsetter.add_path(path, end=True) + # Make sure the path to the repo root is on sys.path. Normally + # it will be, but if we're running this task from another repo + # it might not be. + # + # Note: we can't just set the pythonpath option; that + # option is specifically called out as not being supported + # by robot.run. Plus, robot recommends we call a special + # function instead of directly modifying sys.path + if self.project_config.repo_root not in sys.path: + pythonpathsetter.add_path(self.project_config.repo_root) + num_failed = robot_run(*self.options["suites"], **options) # These numbers are from the robot framework user guide: diff --git a/cumulusci/tasks/robotframework/tests/test_robotframework.py b/cumulusci/tasks/robotframework/tests/test_robotframework.py index 28be70ffca..02c6879a79 100644 --- a/cumulusci/tasks/robotframework/tests/test_robotframework.py +++ b/cumulusci/tasks/robotframework/tests/test_robotframework.py @@ -17,7 +17,8 @@ from cumulusci.tasks.salesforce.tests.util import create_task from cumulusci.tasks.robotframework.debugger import DebugListener from cumulusci.tasks.robotframework.robotframework import KeywordLogger -from cumulusci.utils import touch +from cumulusci.utils import touch, temporary_dir + from cumulusci.tasks.robotframework.libdoc import KeywordFile @@ -246,6 +247,30 @@ def test_sources(self, mock_robot_run): self.assertTrue("dummy1" in sys.path) self.assertTrue("dummy2" in sys.path) + @mock.patch("cumulusci.tasks.robotframework.robotframework.robot_run") + def test_repo_root_in_sys_path(self, mock_robot_run): + """Verify that the repo root is added to sys.path + + Normally, the repo root is added to sys.path in the __init__ + of BaseSalesforceTask. However, if we're running a task from + another repo, the git root of that other repo isn't added. The + robot task will do that; this verifies that. + + """ + mock_robot_run.return_value = 0 + universal_config = UniversalConfig() + project_config = BaseProjectConfig(universal_config) + with temporary_dir() as d: + project_config.repo_info["root"] = d + task = create_task( + Robot, {"suites": "tests"}, project_config=project_config + ) + self.assertNotIn(d, sys.path) + task() + # verify not only that it was added, but that it was added + # at the front of the list. + self.assertEqual(d, sys.path[0]) + @mock.patch("cumulusci.tasks.robotframework.robotframework.robot_run") def test_sources_not_found(self, mock_robot_run): task = create_task( From 1c43b6eca48f618f52b0075cb1e8573d36e503c9 Mon Sep 17 00:00:00 2001 From: Bryan Oakley Date: Wed, 25 Nov 2020 14:59:00 -0600 Subject: [PATCH 2/2] Restore sys.path after calling robot_run --- .../tasks/robotframework/robotframework.py | 8 +++++- .../tests/test_robotframework.py | 26 ++++++++++++------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/cumulusci/tasks/robotframework/robotframework.py b/cumulusci/tasks/robotframework/robotframework.py index 7035705afd..47e2c5a00e 100644 --- a/cumulusci/tasks/robotframework/robotframework.py +++ b/cumulusci/tasks/robotframework/robotframework.py @@ -165,6 +165,9 @@ def _run_task(self): num_failed = result.returncode else: + # Save it so that we can restore it later + orig_sys_path = sys.path.copy() + # Add each source to PYTHONPATH. Robot recommends that we # use pythonpathsetter instead of directly setting # sys.path. @@ -182,7 +185,10 @@ def _run_task(self): if self.project_config.repo_root not in sys.path: pythonpathsetter.add_path(self.project_config.repo_root) - num_failed = robot_run(*self.options["suites"], **options) + try: + num_failed = robot_run(*self.options["suites"], **options) + finally: + sys.path = orig_sys_path # These numbers are from the robot framework user guide: # http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#return-codes diff --git a/cumulusci/tasks/robotframework/tests/test_robotframework.py b/cumulusci/tasks/robotframework/tests/test_robotframework.py index 02c6879a79..8a0d051586 100644 --- a/cumulusci/tasks/robotframework/tests/test_robotframework.py +++ b/cumulusci/tasks/robotframework/tests/test_robotframework.py @@ -210,7 +210,10 @@ def test_user_defined_listeners_option(self): self.assertIn(KeywordLogger, listener_classes) @mock.patch("cumulusci.tasks.robotframework.robotframework.robot_run") - def test_sources(self, mock_robot_run): + @mock.patch( + "cumulusci.tasks.robotframework.robotframework.pythonpathsetter.add_path" + ) + def test_sources(self, mock_add_path, mock_robot_run): """Verify that sources get added to PYTHONPATH when task runs""" universal_config = UniversalConfig() project_config = BaseProjectConfig( @@ -238,17 +241,23 @@ def test_sources(self, mock_robot_run): ) mock_robot_run.return_value = 0 - self.assertFalse("dummy1" in sys.path) - self.assertFalse("dummy2" in sys.path) + self.assertNotIn("dummy1", sys.path) + self.assertNotIn("dummy2", sys.path) task() project_config.get_namespace.assert_has_calls( [mock.call("test1"), mock.call("test2")] ) - self.assertTrue("dummy1" in sys.path) - self.assertTrue("dummy2" in sys.path) + mock_add_path.assert_has_calls( + [mock.call("dummy1", end=True), mock.call("dummy2", end=True)] + ) + self.assertNotIn("dummy1", sys.path) + self.assertNotIn("dummy2", sys.path) @mock.patch("cumulusci.tasks.robotframework.robotframework.robot_run") - def test_repo_root_in_sys_path(self, mock_robot_run): + @mock.patch( + "cumulusci.tasks.robotframework.robotframework.pythonpathsetter.add_path" + ) + def test_repo_root_in_sys_path(self, mock_add_path, mock_robot_run): """Verify that the repo root is added to sys.path Normally, the repo root is added to sys.path in the __init__ @@ -267,9 +276,8 @@ def test_repo_root_in_sys_path(self, mock_robot_run): ) self.assertNotIn(d, sys.path) task() - # verify not only that it was added, but that it was added - # at the front of the list. - self.assertEqual(d, sys.path[0]) + mock_add_path.assert_called_once_with(d) + self.assertNotIn(d, sys.path) @mock.patch("cumulusci.tasks.robotframework.robotframework.robot_run") def test_sources_not_found(self, mock_robot_run):