(graphql-context-test-suite-14) Move some more tests over to the hija…

…cking codepath Summary: Continuing to move code over to the hijacking codepath. Found some weirdness in that we are using check to commmunicate graphql domain errors. Tracking here #2507 Depends on D3087 Test Plan: BK Reviewers: alangenfeld, max Reviewed By: alangenfeld Differential Revision: https://dagster.phacility.com/D3088
dagster-io · May 26, 2020 · 6bed558 · 6bed558
1 parent 3562004
commit 6bed558
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 56 deletions.
diff --git a/python_modules/dagster-graphql/dagster_graphql/schema/roots.py b/python_modules/dagster-graphql/dagster_graphql/schema/roots.py
@@ -499,6 +499,8 @@ def create_execution_params(graphene_info, graphql_execution_params):
         graphene_info.context, graphql_execution_params['selector']
     )
     if preset_name:
+        # This should return proper GraphQL errors
+        # https://github.com/dagster-io/dagster/issues/2507
         check.invariant(
             not graphql_execution_params.get('environmentConfigData'),
             'Invalid ExecutionParams. Cannot define environment_dict when using preset',

diff --git a/python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_execute_pipeline.py b/python_modules/dagster-graphql/dagster_graphql_tests/graphql/test_execute_pipeline.py
@@ -104,65 +104,64 @@ def test_basic_start_pipeline_execution_with_pipeline_def_tags(self, graphql_con
             == 'hello_world_with_tags'
         )
 
+    def test_basic_start_pipeline_execution_with_non_existent_preset(self, graphql_context):
+        with pytest.raises(UserFacingGraphQLError) as exc_info:
+            execute_dagster_graphql(
+                graphql_context,
+                START_PIPELINE_EXECUTION_QUERY,
+                variables={
+                    'executionParams': {
+                        'selector': {'name': 'csv_hello_world'},
+                        'preset': 'undefined_preset',
+                    }
+                },
+            )
 
-def test_basic_start_pipeline_execution_with_non_existent_preset(graphql_context):
-    with pytest.raises(UserFacingGraphQLError) as exc_info:
-        execute_dagster_graphql(
-            graphql_context,
-            START_PIPELINE_EXECUTION_QUERY,
-            variables={
-                'executionParams': {
-                    'selector': {'name': 'csv_hello_world'},
-                    'preset': 'undefined_preset',
-                }
-            },
-        )
-
-    assert (
-        exc_info.value.dauphin_error.message
-        == 'Preset undefined_preset not found in pipeline csv_hello_world.'
-    )
-
-
-def test_basic_start_pipeline_execution_with_preset_failure(graphql_context):
-
-    with pytest.raises(check.CheckError):
-        execute_dagster_graphql(
-            graphql_context,
-            START_PIPELINE_EXECUTION_QUERY,
-            variables={
-                'executionParams': {
-                    'selector': {'name': 'csv_hello_world', 'solidSubset': 'sum_sq_solid'},
-                    'preset': 'test_inline',
-                }
-            },
-        )
-
-    with pytest.raises(check.CheckError):
-        execute_dagster_graphql(
-            graphql_context,
-            START_PIPELINE_EXECUTION_QUERY,
-            variables={
-                'executionParams': {
-                    'selector': {'name': 'csv_hello_world'},
-                    'preset': 'test_inline',
-                    'environmentConfigData': csv_hello_world_solids_config(),
-                }
-            },
+        assert (
+            exc_info.value.dauphin_error.message
+            == 'Preset undefined_preset not found in pipeline csv_hello_world.'
         )
 
-    with pytest.raises(check.CheckError):
-        execute_dagster_graphql(
-            graphql_context,
-            START_PIPELINE_EXECUTION_QUERY,
-            variables={
-                'executionParams': {
-                    'selector': {'name': 'csv_hello_world'},
-                    'preset': 'test_inline',
-                    'mode': 'default',
-                }
-            },
-        )
+    def test_basic_start_pipeline_execution_with_preset_failure(self, graphql_context):
+        # These should check for proper graphql errors rather than check errors
+        # https://github.com/dagster-io/dagster/issues/2507
+        with pytest.raises(check.CheckError):
+            execute_dagster_graphql(
+                graphql_context,
+                START_PIPELINE_EXECUTION_QUERY,
+                variables={
+                    'executionParams': {
+                        'selector': {'name': 'csv_hello_world', 'solidSubset': 'sum_sq_solid'},
+                        'preset': 'test_inline',
+                    }
+                },
+            )
+
+        with pytest.raises(check.CheckError):
+            execute_dagster_graphql(
+                graphql_context,
+                START_PIPELINE_EXECUTION_QUERY,
+                variables={
+                    'executionParams': {
+                        'selector': {'name': 'csv_hello_world'},
+                        'preset': 'test_inline',
+                        'environmentConfigData': csv_hello_world_solids_config(),
+                    }
+                },
+            )
+
+        with pytest.raises(check.CheckError):
+            execute_dagster_graphql(
+                graphql_context,
+                START_PIPELINE_EXECUTION_QUERY,
+                variables={
+                    'executionParams': {
+                        'selector': {'name': 'csv_hello_world'},
+                        'preset': 'test_inline',
+                        'mode': 'default',
+                    }
+                },
+            )
 
 
 def test_basic_start_pipeline_execution_config_failure(graphql_context):