SG-4798, fixes shared cores when site-wide configurations are present. (

#638)

Fixes an issue where a shared core couldn't be used to launch project actions if site wide configurations were found that could be used with that project.

Also fixes a crash when trying to launch an action from a project from a shared core when that project had been moved on disk.

.coveragerc

@@ -16,6 +16,7 @@ source=tank
 omit=
     # We can't unit test our uis, so we're not going to put them inside the coverage
     *tank/authentication/ui/*
+    *tank/authentication/sso_saml2/*
     *tank/platform/qt/*
     *tank/platform/qt5/*
 

.flake8

@@ -43,6 +43,7 @@ exclude =
     # Skip the auto-generated ui file.
     python/tank/authentication/ui/login_dialog.py,
     python/tank/authentication/ui/resources_rc.py,
+    python/tank/authentication/sso_saml2/*,
     tests/python
 
 # toolkit exceptions

python/tank/commands/setup_project.py

@@ -175,7 +175,7 @@ def run_noninteractive(self, log, parameters):
             log.info("Localizing Core...")
             core_localize.do_localize(
                 log,
-                self.tk.shotgun,
+                self._shotgun_connect(log),
                 config_path,
                 suppress_prompts=True
             )
@@ -264,7 +264,7 @@ def run_interactive(self, log, args):
             log.info("Localizing Core...")
             core_localize.do_localize(
                 log,
-                self.tk.shotgun,
+                self._shotgun_connect(log),
                 config_path,
                 suppress_prompts=True
             )

python/tank/pipelineconfig_factory.py

@@ -335,6 +335,12 @@ def _validate_and_create_pipeline_configuration(associated_pipeline_configs, sou
         #       all configurations are always localized
         #
 
+        # We're running with a classic pipeline configuration with shared core, which means we can
+        # ignore site-wide pipeline configurations as they are not compatible with this way of doing
+        # configs. Strip them all out so only project based pipeline configurations remain. This is
+        # important otherwise more than one primary might match a centralized core.
+        primary_pc_data = [pc for pc in primary_pc_data if pc["project_id"]]
+
         if len(primary_pc_data) == 0:
 
             raise TankInitError(
@@ -381,6 +387,20 @@ def _validate_and_create_pipeline_configuration(associated_pipeline_configs, sou
                     "definition in Shotgun." % (sg_config_data["id"], source)
                 )
 
+            # This checks for a very subtle bug. If the toolkit_init.cache contains a path to a
+            # pipeline configuration that matches the pre-requisites, but that doesn't actually
+            # exist on disk because it was either moved to another location or deleted from disk
+            # altogether, then we need to raise TankInitError. If the cache hadn't been force
+            # reread, this will be caught by the factory and Shotgun will be queried once again for
+            # the pipeline configuration info, hopefully finding the real pipeline configuration
+            # this time around.
+            if not os.path.exists(sg_config_data["path"]):
+                raise TankInitError(
+                    "The pipeline configuration %s does not exist on disk. This can happen if the "
+                    "pipeline configuration has been moved to another location or deleted from "
+                    "disk." % sg_config_data["path"]
+                )
+
             # all good. init and return.
             return PipelineConfiguration(sg_config_data["path"])
 

tests/bootstrap_tests/test_configuration_writer.py

@@ -29,7 +29,8 @@ def _write_mock_config(self, shotgun_yml_data=None):
         """
         Creates a fake config with the provided shotgun.yml data.
         """
-        mock_config_root = os.path.join(self.tank_temp, "template", self.id())
+        # Make the file name not too long or we'll run into file length issues on Windows.
+        mock_config_root = os.path.join(self.tank_temp, "template", "%s" % self.short_test_name)
         # Make sure the bundle "exists" on disk.
         os.makedirs(mock_config_root)
 
@@ -49,7 +50,7 @@ def _create_configuration_writer(self):
         """
         Creates a configuration writer that will write to a unique folder for this test.
         """
-        new_config_root = os.path.join(self.tank_temp, "new_configuration", self.id())
+        new_config_root = os.path.join(self.tank_temp, "new_configuration", self.short_test_name)
         shotgun_yml_root = os.path.join(new_config_root, "config", "core")
         # Ensures the location for the shotgun.yml exists.
         os.makedirs(shotgun_yml_root)
@@ -159,7 +160,7 @@ class TestInterpreterFilesWriter(TestConfigurationWriterBase):
     def setUp(self):
         # Makes sure every unit test run in its own sandbox.
         super(TestInterpreterFilesWriter, self).setUp()
-        self._root = os.path.join(self.tank_temp, self.id())
+        self._root = os.path.join(self.tank_temp, self.short_test_name)
         os.makedirs(self._root)
         self._cw = ConfigurationWriter(
             ShotgunPath.from_current_os_path(self._root),
@@ -320,7 +321,7 @@ def _create_test_data(self, create_project):
             dict(type="dev", path="/a/b/c")
         )
 
-        config_root = os.path.join(self.tank_temp, self.id())
+        config_root = os.path.join(self.tank_temp, self.short_test_name)
 
         self.__cw = ConfigurationWriter(
             ShotgunPath.from_current_os_path(config_root),
@@ -506,7 +507,7 @@ def test_transactions(self):
         """
         Ensures the transaction flags are properly handled for a config.
         """
-        new_config_root = os.path.join(self.tank_temp, self.id())
+        new_config_root = os.path.join(self.tank_temp, self.short_test_name)
 
         writer = ConfigurationWriter(
             ShotgunPath.from_current_os_path(new_config_root),

tests/core_tests/test_pipelineconfig_factory.py

@@ -249,6 +249,57 @@ def test_primary_duplicates_from_entity(self):
                                 self.project["id"])
 
 
+class TestSharedCoreWithSiteWideConfigs(TankTestBase):
+
+    def test_multiple_primaries(self):
+        """
+        Ensures that a site-level primary is not considered for a shared-core for a project.
+        """
+        self.mockgun.create(
+            "PipelineConfiguration",
+            {
+                "code": "Primary",
+                "mac_path": "/a/b/c",
+                "windows_path": "C:\\b\\a",
+                "linux_path": "/a/b/c"
+            }
+        )
+
+        sgtk.sgtk_from_path(self.project_root)
+        sgtk.sgtk_from_entity(self.project["type"], self.project["id"])
+
+    def test_no_primary(self):
+        """
+        Ensures error is raised if there are no primary available.
+        """
+        self.mockgun.update(
+            "PipelineConfiguration",
+            self.pipeline_configuration.get_shotgun_id(),
+            {"code": "Secondary"}
+        )
+        with self.assertRaisesRegexp(
+            TankInitError,
+            "does not have a Primary pipeline configuration!"
+        ):
+            sgtk.sgtk_from_path(self.project_root)
+
+    def test_no_path(self):
+        """
+        Ensures error is raised if the primary has no path set.
+        """
+        self.mockgun.update(
+            "PipelineConfiguration",
+            self.pipeline_configuration.get_shotgun_id(),
+            {"windows_path": None, "linux_path": None, "mac_path": None}
+        )
+        # We do not support site-wide pipeline configurations from shared cores.
+        with self.assertRaisesRegexp(
+            TankInitError,
+            "cannot be instantiated because it does not have an absolute path"
+        ):
+            sgtk.sgtk_from_path(self.project_root)
+
+
 class TestPipelineConfigurationEnumeration(ShotgunTestBase):
     """
     Tests pipeline configuration enumeration.
@@ -468,7 +519,6 @@ class TestTankFromWithSiteConfig(TankTestBase):
     """
     Tests tank.tank_from_* with site configurations.
     """
-
     def setUp(self):
         super(TestTankFromWithSiteConfig, self).setUp()
         # Turn the config into a site configuration.
@@ -500,6 +550,13 @@ def test_from_path(self):
             result = tank.tank_from_path(self.project_root)
             self.assertEquals(result.project_path, self.project_root)
             self.assertEquals(result.pipeline_configuration.get_path(), self.pipeline_config_root)
+
+            self._invalidate_pipeline_configuration_yml()
+            with self.assertRaisesRegexp(
+                TankInitError,
+                "however that is not associated with the pipeline configuration"
+            ):
+                tank.tank_from_path(self.project_root)
         finally:
             del os.environ["TANK_CURRENT_PC"]
 
@@ -512,9 +569,33 @@ def test_from_entity(self):
             result = tank.tank_from_entity("Project", self.project["id"])
             self.assertEquals(result.project_path, self.project_root)
             self.assertEquals(result.pipeline_configuration.get_path(), self.pipeline_config_root)
+
+            self._invalidate_pipeline_configuration_yml()
+            with self.assertRaisesRegexp(
+                TankInitError,
+                "however that is not associated with the pipeline configuration"
+            ):
+                tank.tank_from_entity("Project", self.project["id"])
         finally:
             del os.environ["TANK_CURRENT_PC"]
 
+    def _invalidate_pipeline_configuration_yml(self):
+        """
+        Updates pipeline_configuration.yml to point to a pipeline configuration id
+        that doesn't match.
+        """
+        pc_yml = os.path.join(self.pipeline_config_root, "config", "core", "pipeline_configuration.yml")
+        pc_yml_data = (
+            "{ project_name: %s, use_shotgun_path_cache: true, pc_id: %d, "
+            "project_id: %d, pc_name: %s}\n\n" % (
+                self.project["tank_name"],
+                9595,
+                self.project["id"],
+                self.sg_pc_entity["code"]
+            )
+        )
+        self.create_file(pc_yml, pc_yml_data)
+
 
 class TestTankFromEntityWithMixedSlashes(TankTestBase):
     """

tests/integration_tests/README.md

@@ -8,25 +8,26 @@ site.
 
 These variables, which are also set on our CI servers, should be set before running the tests.
 
-``SHOTGUN_HOST``: URL of the live site to use to run the tests.
-``SHOTGUN_SCRIPT_NAME``: Name of script on that site.
-``SHOTGUN_SCRIPT_KEY``: Key for said script.
+`SHOTGUN_HOST`: URL of the live site to use to run the tests.
+`SHOTGUN_SCRIPT_NAME`: Name of script on that site.
+`SHOTGUN_SCRIPT_KEY`: Key for said script.
 
 How to run an integration test
 ------------------------------
-Simply set the three environment variables and then type ``python <test_name>.py``.
+ - set the three environment variables
+ - add `<tk-core>/python` and `<tk-core>/tests/python` in the `PYTHONPATH`
+ - launch a test by typing `python <test_name>.py`.
 
 How to write an integration test
 --------------------------------
 
-There is currently no framework to write those. You should split your test
-in multiple sub-tests that can be run independently, so you can run them one at
-a time. You can leverage pyUnit's functionality for that.
+There is a special class to write integration tests named SgtkIntegrationTest and is importable through
 
-What's currently in the first test needs to be refactored to be reusable for other tests. Keep that in mind if you are adding a new test.
+    from sgtk_integration_test import SgtkIntegrationTest
 
-Note that pyUnit's documentation says that tests in a tests class are sorted
-alphabetically, so you can number your tests functions to order them.
+This test class will take care of sandboxing your test in a way that multiple continuous integration
+should be able to run in parallel without having the tests step on each other's toes. See `SgtkIntegrationTest`s
+documentation to learn more how the class can help you sandbox tests.
 
 Future work
 -----------
@@ -40,5 +41,9 @@ The pre-requisites for picking a framework would have to be:
 - be open-source or publicly available, as we want this to run on travis
 - be able to split a test in multiple steps, which can fail the test early
 - be able to run a test one step at a time to debug them
+    - this is complicated right now, you need to comment out the `safe_delete_folder` call in the
+      base integration class and add an underscore in front of steps you want to skip.
 - make it easy to integrate code coverage from subprocesses that are invoked
+    - right now the coverage is captured but can't be merged, as you can see from travis's coveralls
+      command.
 - make it easy to have a single test folder for multiple tests that is cleaned up on exit

tests/integration_tests/data/offline_workflow_config/core/core_api.yml → tests/integration_tests/data/simple_config/core/core_api.yml

@@ -10,6 +10,5 @@
 #
 
 location:
-    type: app_store
-    name: tk-core
-    version: v0.18.133
+    type: path
+    path: $TK_CORE_REPO_ROOT

tests/integration_tests/data/simple_config/core/roots.yml

@@ -0,0 +1,13 @@
+# Copyright (c) 2018 Shotgun Software Inc.
+#
+# CONFIDENTIAL AND PROPRIETARY
+#
+# This work is provided "AS IS" and subject to the Shotgun Pipeline Toolkit
+# Source Code License included in this distribution package. See LICENSE.
+# By accessing, using, copying or modifying this work you indicate your
+# agreement to the Shotgun Pipeline Toolkit Source Code License. All rights
+# not expressly granted therein are reserved by Shotgun Software Inc.
+
+# Leave this file empty. Legacy site config has an empty roots file because it
+# didn't use any storage.
+integration_tests: {}

tests/integration_tests/data/simple_config/hooks/placeholder

@@ -0,0 +1,11 @@
+# Copyright (c) 2018 Shotgun Software Inc.
+#
+# CONFIDENTIAL AND PROPRIETARY
+#
+# This work is provided "AS IS" and subject to the Shotgun Pipeline Toolkit
+# Source Code License included in this distribution package. See LICENSE.
+# By accessing, using, copying or modifying this work you indicate your
+# agreement to the Shotgun Pipeline Toolkit Source Code License. All rights
+# not expressly granted therein are reserved by Shotgun Software Inc.
+
+# Required to setup_project can work.

tests/integration_tests/data/site_config/core/core_api.yml

@@ -0,0 +1,14 @@
+# Copyright (c) 2018 Shotgun Software Inc.
+#
+# CONFIDENTIAL AND PROPRIETARY
+#
+# This work is provided "AS IS" and subject to the Shotgun Pipeline Toolkit
+# Source Code License included in this distribution package. See LICENSE.
+# By accessing, using, copying or modifying this work you indicate your
+# agreement to the Shotgun Pipeline Toolkit Source Code License. All rights
+# not expressly granted therein are reserved by Shotgun Software Inc.
+#
+
+location:
+    type: path
+    path: $TK_CORE_REPO_ROOT

tests/integration_tests/data/site_config/core/hooks/pick_environment.py

@@ -0,0 +1,23 @@
+# Copyright (c) 2018 Shotgun Software Inc.
+#
+# CONFIDENTIAL AND PROPRIETARY
+#
+# This work is provided "AS IS" and subject to the Shotgun Pipeline Toolkit
+# Source Code License included in this distribution package. See LICENSE.
+# By accessing, using, copying or modifying this work you indicate your
+# agreement to the Shotgun Pipeline Toolkit Source Code License. All rights
+# not expressly granted therein are reserved by Shotgun Software Inc.
+
+from sgtk import Hook
+
+
+class PickEnvironment(Hook):
+    """
+    The pick environment hook gets called when Toolkit tries to
+    determine which configuration to use. Based on the context
+    provided by Toolkit, the name of the environment file to be
+    used should be returned by the execute method below.
+    """
+
+    def execute(self, context, **kwargs):
+        return "project"

tests/integration_tests/data/site_config/core/roots.yml

@@ -0,0 +1,2 @@
+# Leave this file empty. Legacy site config has an empty roots file because it
+# didn't use any storage.

tests/integration_tests/data/site_config/env/project.yml

@@ -0,0 +1,39 @@
+# Copyright (c) 2018 Shotgun Software Inc.
+#
+# CONFIDENTIAL AND PROPRIETARY
+#
+# This work is provided "AS IS" and subject to the Shotgun Pipeline Toolkit
+# Source Code License included in this distribution package. See LICENSE.
+# By accessing, using, copying or modifying this work you indicate your
+# agreement to the Shotgun Pipeline Toolkit Source Code License. All rights
+# not expressly granted therein are reserved by Shotgun Software Inc.
+#
+
+description: Small environment to test the offline workflow.
+
+################################################################################
+
+
+################################################################################
+# configuration for all engines to load in a project context
+
+engines:
+  tk-shell:
+    apps:
+      tk-multi-launchapp:
+        use_software_entity: true
+        location: 
+          type: app_store
+          name: tk-multi-launchapp
+          version: v0.9.15
+    location:
+      type: app_store
+      name: tk-shell
+      version: v0.6.0
+
+frameworks:
+  tk-framework-shotgunutils_v5.x.x:
+    location:
+      version: v5.3.4
+      type: app_store
+      name: tk-framework-shotgunutils