Implement missing execution test.

* Implements the test "does not include arguments that were not set".

In doing this, I realized that the `get_argument_values` function was improperly implemented. I also noticed that `coerce_value_ast` is `value_from_ast` utility.

Changing `get_argument_values` broke a lot of tests that should have used .get() when expecting a Nullable value.

Author: jhgg

graphql/core/execution/values.py

@@ -1,7 +1,6 @@
 import collections
 from ..compat import str_type
 from ..error import GraphQLError
-from ..language import ast
 from ..language.printer import print_ast
 from ..type import (
     GraphQLEnumType,
@@ -13,6 +12,7 @@
 )
 from ..utils.is_nullish import is_nullish
 from ..utils.type_from_ast import type_from_ast
+from ..utils.value_from_ast import value_from_ast
 
 __all__ = ['get_variable_values', 'get_argument_values']
 
@@ -33,29 +33,37 @@ def get_variable_values(schema, definition_asts, inputs):
 def get_argument_values(arg_defs, arg_asts, variables):
     """Prepares an object map of argument values given a list of argument
     definitions and list of argument AST nodes."""
-    arg_ast_map = {}
     if arg_asts:
-        for arg in arg_asts:
-            arg_ast_map[arg.name.value] = arg
+        arg_ast_map = {arg.name.value: arg for arg in arg_asts}
+
+    else:
+        arg_ast_map = {}
+
     result = {}
     for arg_def in arg_defs:
         name = arg_def.name
         value_ast = arg_ast_map.get(name)
         if value_ast:
             value_ast = value_ast.value
-        value = coerce_value_ast(
+
+        value = value_from_ast(
             arg_def.type,
             value_ast,
             variables
         )
-        if is_nullish(value) and not is_nullish(arg_def.default_value):
+
+        if is_nullish(value):
             value = arg_def.default_value
-        result[name] = value
+
+        if not is_nullish(value):
+            result[name] = value
+
     return result
 
 
 def get_variable_value(schema, definition_ast, input):
-    """Given a variable definition, and any value of input, return a value which adheres to the variable definition, or throw an error."""
+    """Given a variable definition, and any value of input, return a value which adheres to the variable definition,
+    or throw an error."""
     type = type_from_ast(schema, definition_ast.type)
     if not type or not is_input_type(type):
         raise GraphQLError(
@@ -65,12 +73,14 @@ def get_variable_value(schema, definition_ast, input):
             ),
             [definition_ast]
         )
+
     if is_valid_value(type, input):
         if is_nullish(input):
             default_value = definition_ast.default_value
             if default_value:
-                return coerce_value_ast(type, default_value, None)
+                return value_from_ast(type, default_value, None)
         return coerce_value(type, input)
+
     raise GraphQLError(
         'Variable ${} expected value of type {} but got: {}'.format(
             definition_ast.variable.name.value,
@@ -156,61 +166,3 @@ def coerce_value(type, value):
         return parsed
 
     return None
-
-
-def coerce_value_ast(type, value_ast, variables):
-    """Given a type and a value AST node known to match this type, build a
-    runtime value."""
-    if isinstance(type, GraphQLNonNull):
-        # Note: we're not checking that the result of coerceValueAST is non-null.
-        # We're assuming that this query has been validated and the value used here is of the correct type.
-        return coerce_value_ast(type.of_type, value_ast, variables)
-
-    if not value_ast:
-        return None
-
-    if isinstance(value_ast, ast.Variable):
-        variable_name = value_ast.name.value
-        if not variables or variable_name not in variables:
-            return None
-        # Note: we're not doing any checking that this variable is correct. We're assuming that this query
-        # has been validated and the variable usage here is of the correct type.
-        return variables[variable_name]
-
-    if isinstance(type, GraphQLList):
-        item_type = type.of_type
-        if isinstance(value_ast, ast.ListValue):
-            return [coerce_value_ast(item_type, item_ast, variables)
-                    for item_ast in value_ast.values]
-        else:
-            return [coerce_value_ast(item_type, value_ast, variables)]
-
-    if isinstance(type, GraphQLInputObjectType):
-        fields = type.get_fields()
-        if not isinstance(value_ast, ast.ObjectValue):
-            return None
-        field_asts = {}
-        for field in value_ast.fields:
-            field_asts[field.name.value] = field
-        obj = {}
-        for field_name, field in fields.items():
-            field_ast = field_asts.get(field_name)
-            field_value_ast = None
-            if field_ast:
-                field_value_ast = field_ast.value
-            field_value = coerce_value_ast(
-                field.type, field_value_ast, variables
-            )
-            if field_value is None:
-                field_value = field.default_value
-            obj[field_name] = field_value
-        return obj
-
-    assert isinstance(type, (GraphQLScalarType, GraphQLEnumType)), \
-        'Must be input type'
-
-    parsed = type.parse_literal(value_ast)
-    if not is_nullish(parsed):
-        return parsed
-
-    return None

graphql/core/utils/value_from_ast.py

@@ -0,0 +1,61 @@
+from ..language import ast
+from ..type import (GraphQLEnumType, GraphQLInputObjectType, GraphQLList, GraphQLNonNull, GraphQLScalarType)
+from .is_nullish import is_nullish
+
+
+def value_from_ast(type, value_ast, variables):
+    """Given a type and a value AST node known to match this type, build a
+    runtime value."""
+    if isinstance(type, GraphQLNonNull):
+        # Note: we're not checking that the result of coerceValueAST is non-null.
+        # We're assuming that this query has been validated and the value used here is of the correct type.
+        return value_from_ast(type.of_type, value_ast, variables)
+
+    if not value_ast:
+        return None
+
+    if isinstance(value_ast, ast.Variable):
+        variable_name = value_ast.name.value
+        if not variables or variable_name not in variables:
+            return None
+        # Note: we're not doing any checking that this variable is correct. We're assuming that this query
+        # has been validated and the variable usage here is of the correct type.
+        return variables[variable_name]
+
+    if isinstance(type, GraphQLList):
+        item_type = type.of_type
+        if isinstance(value_ast, ast.ListValue):
+            return [value_from_ast(item_type, item_ast, variables)
+                    for item_ast in value_ast.values]
+        else:
+            return [value_from_ast(item_type, value_ast, variables)]
+
+    if isinstance(type, GraphQLInputObjectType):
+        fields = type.get_fields()
+        if not isinstance(value_ast, ast.ObjectValue):
+            return None
+        field_asts = {}
+        for field in value_ast.fields:
+            field_asts[field.name.value] = field
+        obj = {}
+        for field_name, field in fields.items():
+            field_ast = field_asts.get(field_name)
+            field_value_ast = None
+            if field_ast:
+                field_value_ast = field_ast.value
+            field_value = value_from_ast(
+                field.type, field_value_ast, variables
+            )
+            if field_value is None:
+                field_value = field.default_value
+            obj[field_name] = field_value
+        return obj
+
+    assert isinstance(type, (GraphQLScalarType, GraphQLEnumType)), \
+        'Must be input type'
+
+    parsed = type.parse_literal(value_ast)
+    if not is_nullish(parsed):
+        return parsed
+
+    return None

tests/core_execution/test_executor.py

@@ -1,8 +1,10 @@
+import json
 from pytest import raises
 from graphql.core.execution import execute
 from graphql.core.language.parser import parse
 from graphql.core.type import (GraphQLSchema, GraphQLObjectType, GraphQLField,
-    GraphQLArgument, GraphQLList, GraphQLInt, GraphQLString)
+                               GraphQLArgument, GraphQLList, GraphQLInt, GraphQLString,
+                               GraphQLBoolean)
 from graphql.core.error import GraphQLError
 
 
@@ -29,7 +31,7 @@ class DeepData(object):
         a = 'Already Been Done'
         b = 'Boring'
         c = ['Contrived', None, 'Confusing']
-        
+
         def deeper(self):
             return [Data(), None, Data()]
 
@@ -71,15 +73,15 @@ def deeper(self):
         'e': 'Egg',
         'f': 'Fish',
         'pic': 'Pic of size: 100',
-        'promise': { 'a': 'Apple' },
+        'promise': {'a': 'Apple'},
         'deep': {
-          'a': 'Already Been Done',
-          'b': 'Boring',
-          'c': [ 'Contrived', None, 'Confusing' ],
-          'deeper': [
-            { 'a': 'Apple', 'b': 'Banana' },
-            None,
-            { 'a': 'Apple', 'b': 'Banana' } ] }
+            'a': 'Already Been Done',
+            'b': 'Boring',
+            'c': ['Contrived', None, 'Confusing'],
+            'deeper': [
+                {'a': 'Apple', 'b': 'Banana'},
+                None,
+                {'a': 'Apple', 'b': 'Banana'}]}
     }
 
     DataType = GraphQLObjectType('DataType', lambda: {
@@ -141,17 +143,17 @@ def test_merges_parallel_fragments():
     result = execute(schema, None, ast)
     assert not result.errors
     assert result.data == \
-        {
-            'a': 'Apple',
-            'b': 'Banana',
-            'c': 'Cherry',
-            'deep': {
-              'b': 'Banana',
-              'c': 'Cherry',
-              'deeper': {
-                'b': 'Banana',
-                'c': 'Cherry' } }
-        }
+           {
+               'a': 'Apple',
+               'b': 'Banana',
+               'c': 'Cherry',
+               'deep': {
+                   'b': 'Banana',
+                   'c': 'Cherry',
+                   'deeper': {
+                       'b': 'Banana',
+                       'c': 'Cherry'}}
+           }
 
 
 def test_threads_context_correctly():
@@ -165,7 +167,7 @@ class Data(object):
     def resolver(context, *_):
         assert context.context_thing == 'thing'
         resolver.got_here = True
-    
+
     resolver.got_here = False
 
     Type = GraphQLObjectType('Type', {
@@ -237,8 +239,10 @@ def error(self):
 
 def test_uses_the_inline_operation_if_no_operation_is_provided():
     doc = '{ a }'
+
     class Data(object):
         a = 'b'
+
     ast = parse(doc)
     Type = GraphQLObjectType('Type', {
         'a': GraphQLField(GraphQLString)
@@ -250,8 +254,10 @@ class Data(object):
 
 def test_uses_the_only_operation_if_no_operation_is_provided():
     doc = 'query Example { a }'
+
     class Data(object):
         a = 'b'
+
     ast = parse(doc)
     Type = GraphQLObjectType('Type', {
         'a': GraphQLField(GraphQLString)
@@ -263,8 +269,10 @@ class Data(object):
 
 def test_raises_the_inline_operation_if_no_operation_is_provided():
     doc = 'query Example { a } query OtherExample { a }'
+
     class Data(object):
         a = 'b'
+
     ast = parse(doc)
     Type = GraphQLObjectType('Type', {
         'a': GraphQLField(GraphQLString)
@@ -276,9 +284,11 @@ class Data(object):
 
 def test_uses_the_query_schema_for_queries():
     doc = 'query Q { a } mutation M { c }'
+
     class Data(object):
         a = 'b'
         c = 'd'
+
     ast = parse(doc)
     Q = GraphQLObjectType('Q', {
         'a': GraphQLField(GraphQLString)
@@ -293,9 +303,11 @@ class Data(object):
 
 def test_uses_the_mutation_schema_for_queries():
     doc = 'query Q { a } mutation M { c }'
+
     class Data(object):
         a = 'b'
         c = 'd'
+
     ast = parse(doc)
     Q = GraphQLObjectType('Q', {
         'a': GraphQLField(GraphQLString)
@@ -320,8 +332,10 @@ def test_avoids_recursion():
             ...Frag
         }
     '''
+
     class Data(object):
         a = 'b'
+
     ast = parse(doc)
     Type = GraphQLObjectType('Type', {
         'a': GraphQLField(GraphQLString)
@@ -343,3 +357,28 @@ def test_does_not_include_illegal_fields_in_output():
     result = execute(GraphQLSchema(Q, M), None, ast)
     assert not result.errors
     assert result.data == {}
+
+
+def test_does_not_include_arguments_that_were_not_set():
+    schema = GraphQLSchema(GraphQLObjectType(
+        'Type',
+        {
+            'field': GraphQLField(
+                GraphQLString,
+                resolver=lambda data, args, *_: args and json.dumps(args, sort_keys=True, separators=(',',':')),
+                args={
+                    'a': GraphQLArgument(GraphQLBoolean),
+                    'b': GraphQLArgument(GraphQLBoolean),
+                    'c': GraphQLArgument(GraphQLBoolean),
+                    'd': GraphQLArgument(GraphQLInt),
+                    'e': GraphQLArgument(GraphQLInt),
+                }
+            )
+        }
+    ))
+
+    ast = parse('{ field(a: true, c: false, e: 0) }')
+    result = execute(schema, None, ast)
+    assert result.data == {
+        'field': '{"a":true,"c":false,"e":0}'
+    }