From 91aee7fdeb111a1f59cd5e023972c84d406c2d32 Mon Sep 17 00:00:00 2001 From: Brandon Simmons Date: Wed, 4 Sep 2019 11:02:35 -0400 Subject: [PATCH] Test result ordering, add `--accept` test mode to automatically accept changed test cases We add a new pytest flag `--accept` that will automatically write back yaml files with updated responses. This makes it much easier and less error-prone to update test cases when we expect output to change, or when authoring new tests. Second we make sure to test that we actually preserve the order of the selection set when returning results. This is a "SHOULD" part of the spec but seems pretty important and something that users will rely on. To support both of the above we use ruamel.yaml which preserves a certain amount of formatting and comments (so that --accept can work in a failry ergonomic way), as well as ordering (so that when we write yaml the order of keys has meaning that's preserved during parsing). Use ruamel.yaml everywhere for consistency (since both libraries have different quirks). Quirks of ruamel.yaml: - trailing whitespace in multiline strings in yaml files isn't written back out as we'd like: https://bitbucket.org/ruamel/yaml/issues/47/multiline-strings-being-changed-if-they - formatting is only sort of preserved; ruamel e.g. normalizes indentation. Normally the diff is pretty clean though, and you can always just check in portions of your test file after --accept fixup --- server/tests-py/conftest.py | 8 + server/tests-py/context.py | 19 +- server/tests-py/pytest.ini | 4 + server/tests-py/requirements-top-level.txt | 2 + server/tests-py/requirements.txt | 1 + server/tests-py/test_apis_disabled.py | 2 +- server/tests-py/test_compression.py | 5 +- server/tests-py/test_config_api.py | 2 +- server/tests-py/test_events.py | 2 +- server/tests-py/test_graphql_introspection.py | 4 +- server/tests-py/test_graphql_mutations.py | 2 +- server/tests-py/test_graphql_queries.py | 2 +- server/tests-py/test_horizontal_scale.py | 6 +- server/tests-py/test_inconsistent_meta.py | 8 +- server/tests-py/test_jwt.py | 2 +- server/tests-py/test_pg_dump.py | 2 +- server/tests-py/test_schema_stitching.py | 8 +- server/tests-py/test_subscriptions.py | 2 +- server/tests-py/test_tests.py | 160 +++++++++++++++++ ...ect_query_author_by_pkey_bad_ordering.yaml | 17 ++ ...n_query_jsonb_values_filter_bad_order.yaml | 21 +++ ...query_jsonb_values_filter_okay_orders.yaml | 42 +++++ server/tests-py/test_v1_queries.py | 2 +- server/tests-py/test_v1alpha1_endpoint.py | 2 +- server/tests-py/test_validation.py | 2 +- server/tests-py/validate.py | 168 +++++++++++++++--- 26 files changed, 435 insertions(+), 60 deletions(-) create mode 100755 server/tests-py/test_tests.py create mode 100644 server/tests-py/test_tests/select_query_author_by_pkey_bad_ordering.yaml create mode 100644 server/tests-py/test_tests/user_can_query_jsonb_values_filter_bad_order.yaml create mode 100644 server/tests-py/test_tests/user_can_query_jsonb_values_filter_okay_orders.yaml diff --git a/server/tests-py/conftest.py b/server/tests-py/conftest.py index 74a14f91..408af263 100644 --- a/server/tests-py/conftest.py +++ b/server/tests-py/conftest.py @@ -81,6 +81,14 @@ def pytest_addoption(parser): help="Run testcases for logging" ) + parser.addoption( + "--accept", + action="store_true", + default=False, + required=False, + help="Accept any failing test cases from YAML files as correct, and write the new files out to disk." + ) + #By default, #1) Set default parallelism to one diff --git a/server/tests-py/context.py b/server/tests-py/context.py index f21b729a..bf988c8f 100644 --- a/server/tests-py/context.py +++ b/server/tests-py/context.py @@ -2,6 +2,8 @@ from http import HTTPStatus from urllib.parse import urlparse +from ruamel.yaml.comments import CommentedMap as OrderedDict # to avoid '!!omap' in yaml +# from collections import OrderedDict # import socketserver import threading import http.server @@ -14,7 +16,7 @@ import uuid import string import random -import yaml +import ruamel.yaml as yaml import requests import websocket from sqlalchemy import create_engine @@ -128,7 +130,9 @@ class GQLWsClient(): self.connected_event.set() def _on_message(self, message): - json_msg = json.loads(message) + # NOTE: make sure we preserve key ordering so we can test the ordering + # properties in the graphql spec properly + json_msg = json.loads(message, object_pairs_hook=OrderedDict) if 'id' in json_msg: query_id = json_msg['id'] if json_msg.get('type') == 'stop': @@ -279,7 +283,9 @@ class HGECtx: json=q, headers=h ) - return resp.status_code, resp.json() + # NOTE: make sure we preserve key ordering so we can test the ordering + # properties in the graphql spec properly + return resp.status_code, resp.json(object_pairs_hook=OrderedDict) def sql(self, q): conn = self.engine.connect() @@ -296,11 +302,14 @@ class HGECtx: json=q, headers=h ) - return resp.status_code, resp.json() + # NOTE: make sure we preserve key ordering so we can test the ordering + # properties in the graphql spec properly + return resp.status_code, resp.json(object_pairs_hook=OrderedDict) def v1q_f(self, fn): with open(fn) as f: - return self.v1q(yaml.safe_load(f)) + # NOTE: preserve ordering with RoundTripLoader: + return self.v1q(yaml.load(f, yaml.RoundTripLoader)) def teardown(self): self.http.close() diff --git a/server/tests-py/pytest.ini b/server/tests-py/pytest.ini index 1b6eec80..ddd6db63 100644 --- a/server/tests-py/pytest.ini +++ b/server/tests-py/pytest.ini @@ -1,2 +1,6 @@ [pytest] norecursedirs = queries webhook +; Turn any expected failures that pass ("xpassed") into hard failures. This +; lets us use 'xfail' to create test cases that validate other tests, and also +; means we're sure to notice if e.g. a known bug is fixed. +xfail_strict = true diff --git a/server/tests-py/requirements-top-level.txt b/server/tests-py/requirements-top-level.txt index 6b5ca6e6..41a40eeb 100644 --- a/server/tests-py/requirements-top-level.txt +++ b/server/tests-py/requirements-top-level.txt @@ -10,3 +10,5 @@ jsondiff cryptography graphene brotlipy +ruamel.yaml < 0.15 +graphql-core diff --git a/server/tests-py/requirements.txt b/server/tests-py/requirements.txt index dea895c5..0c879121 100644 --- a/server/tests-py/requirements.txt +++ b/server/tests-py/requirements.txt @@ -34,3 +34,4 @@ wcwidth==0.1.7 websocket-client==0.56.0 zipp==0.5.1 brotlipy==0.7.0 +ruamel.yaml==0.14.12 diff --git a/server/tests-py/test_apis_disabled.py b/server/tests-py/test_apis_disabled.py index 8639c9ee..9d12acfd 100644 --- a/server/tests-py/test_apis_disabled.py +++ b/server/tests-py/test_apis_disabled.py @@ -8,7 +8,7 @@ def check_post_404(hge_ctx,url): 'url': url, 'status': 404, 'query': {} - }) + })[0] @pytest.mark.skipif(not pytest.config.getoption("--test-metadata-disabled"), diff --git a/server/tests-py/test_compression.py b/server/tests-py/test_compression.py index 8d8da452..74a80707 100644 --- a/server/tests-py/test_compression.py +++ b/server/tests-py/test_compression.py @@ -1,11 +1,10 @@ #!/usr/bin/env python3 import pytest -import yaml +import ruamel.yaml as yaml import jsondiff from super_classes import DefaultTestSelectQueries -from validate import json_ordered class TestCompression(DefaultTestSelectQueries): @@ -37,7 +36,7 @@ class TestCompression(DefaultTestSelectQueries): def _assert_resp(self, resp, exp_resp): json_resp = resp.json() - assert json_ordered(json_resp) == json_ordered(exp_resp), yaml.dump({ + assert json_resp == exp_resp, yaml.dump({ 'response': json_resp, 'expected': exp_resp, 'diff': jsondiff.diff(exp_resp, json_resp) diff --git a/server/tests-py/test_config_api.py b/server/tests-py/test_config_api.py index 19efb5fc..8d2c1723 100644 --- a/server/tests-py/test_config_api.py +++ b/server/tests-py/test_config_api.py @@ -1,4 +1,4 @@ -import yaml +import ruamel.yaml as yaml import re class TestConfigAPI(): diff --git a/server/tests-py/test_events.py b/server/tests-py/test_events.py index ea495bc4..c1ef14f0 100755 --- a/server/tests-py/test_events.py +++ b/server/tests-py/test_events.py @@ -2,7 +2,7 @@ import pytest import queue -import yaml +import ruamel.yaml as yaml import time from super_classes import DefaultTestQueries from validate import check_query_f, check_query, check_event diff --git a/server/tests-py/test_graphql_introspection.py b/server/tests-py/test_graphql_introspection.py index 2f3307d9..ec0b54ec 100644 --- a/server/tests-py/test_graphql_introspection.py +++ b/server/tests-py/test_graphql_introspection.py @@ -1,4 +1,4 @@ -import yaml +import ruamel.yaml as yaml from validate import check_query_f, check_query from super_classes import DefaultTestSelectQueries @@ -8,7 +8,7 @@ class TestGraphqlIntrospection(DefaultTestSelectQueries): def test_introspection(self, hge_ctx): with open(self.dir() + "/introspection.yaml") as c: conf = yaml.safe_load(c) - resp = check_query(hge_ctx, conf) + resp, _ = check_query(hge_ctx, conf) hasArticle = False hasArticleAuthorFKRel = False hasArticleAuthorManualRel = False diff --git a/server/tests-py/test_graphql_mutations.py b/server/tests-py/test_graphql_mutations.py index c662fbab..193e3635 100644 --- a/server/tests-py/test_graphql_mutations.py +++ b/server/tests-py/test_graphql_mutations.py @@ -1,5 +1,5 @@ import pytest -import yaml +import ruamel.yaml as yaml from validate import check_query_f from super_classes import DefaultTestQueries, DefaultTestMutations diff --git a/server/tests-py/test_graphql_queries.py b/server/tests-py/test_graphql_queries.py index 6aa216da..39a14ea5 100644 --- a/server/tests-py/test_graphql_queries.py +++ b/server/tests-py/test_graphql_queries.py @@ -1,4 +1,4 @@ -import yaml +import ruamel.yaml as yaml import pytest from validate import check_query_f from super_classes import DefaultTestSelectQueries diff --git a/server/tests-py/test_horizontal_scale.py b/server/tests-py/test_horizontal_scale.py index e64b74da..ad5c740a 100644 --- a/server/tests-py/test_horizontal_scale.py +++ b/server/tests-py/test_horizontal_scale.py @@ -1,10 +1,8 @@ import pytest -import yaml +import ruamel.yaml as yaml import time import jsondiff -from validate import json_ordered - if not pytest.config.getoption("--test-hge-scale-url"): pytest.skip("--test-hge-scale-url flag is missing, skipping tests", allow_module_level=True) @@ -49,7 +47,7 @@ class TestHorizantalScaleBasic(): assert st_code == 200, resp if 'response' in step['validate']: - assert json_ordered(resp) == json_ordered(step['validate']['response']), yaml.dump({ + assert resp == step['validate']['response'], yaml.dump({ 'response': resp, 'expected': step['validate']['response'], 'diff': jsondiff.diff(step['validate']['response'], resp) diff --git a/server/tests-py/test_inconsistent_meta.py b/server/tests-py/test_inconsistent_meta.py index 1af8e841..f2c6e568 100644 --- a/server/tests-py/test_inconsistent_meta.py +++ b/server/tests-py/test_inconsistent_meta.py @@ -1,10 +1,8 @@ import pytest -import yaml +import ruamel.yaml as yaml import json import jsondiff -from validate import json_ordered - class TestInconsistentObjects(): get_inconsistent_metadata = { @@ -26,7 +24,7 @@ class TestInconsistentObjects(): def test_inconsistent_objects(self, hge_ctx): with open(self.dir() + "/test.yaml") as c: - test = yaml.load(c) + test = yaml.safe_load(c) # setup st_code, resp = hge_ctx.v1q(json.loads(json.dumps(test['setup']))) @@ -46,7 +44,7 @@ class TestInconsistentObjects(): incons_objs_resp = resp['inconsistent_objects'] assert resp['is_consistent'] == False, resp - assert json_ordered(incons_objs_resp) == json_ordered(incons_objs_test), yaml.dump({ + assert incons_objs_resp == incons_objs_test, yaml.dump({ 'response': resp, 'expected': incons_objs_test, 'diff': jsondiff.diff(incons_objs_test, resp) diff --git a/server/tests-py/test_jwt.py b/server/tests-py/test_jwt.py index 257b2626..71c6f5f9 100644 --- a/server/tests-py/test_jwt.py +++ b/server/tests-py/test_jwt.py @@ -3,7 +3,7 @@ import math import json import time -import yaml +import ruamel.yaml as yaml import pytest import jwt from test_subscriptions import init_ws_conn diff --git a/server/tests-py/test_pg_dump.py b/server/tests-py/test_pg_dump.py index 238832e7..7e9f4836 100644 --- a/server/tests-py/test_pg_dump.py +++ b/server/tests-py/test_pg_dump.py @@ -1,4 +1,4 @@ -import yaml +import ruamel.yaml as yaml from super_classes import DefaultTestSelectQueries import os diff --git a/server/tests-py/test_schema_stitching.py b/server/tests-py/test_schema_stitching.py index 22daad12..06dc4995 100644 --- a/server/tests-py/test_schema_stitching.py +++ b/server/tests-py/test_schema_stitching.py @@ -2,7 +2,7 @@ import string import random -import yaml +import ruamel.yaml as yaml import json import queue import requests @@ -71,7 +71,7 @@ class TestRemoteSchemaBasic: #check_query_f(hge_ctx, 'queries/graphql_introspection/introspection.yaml') with open('queries/graphql_introspection/introspection.yaml') as f: query = yaml.safe_load(f) - resp = check_query(hge_ctx, query) + resp, _ = check_query(hge_ctx, query) assert check_introspection_result(resp, ['Hello'], ['hello']) # @@ -235,7 +235,7 @@ class TestAddRemoteSchemaTbls: def test_introspection(self, hge_ctx): with open('queries/graphql_introspection/introspection.yaml') as f: query = yaml.safe_load(f) - resp = check_query(hge_ctx, query) + resp, _ = check_query(hge_ctx, query) assert check_introspection_result(resp, ['User', 'hello'], ['user', 'hello']) def test_add_schema_duplicate_name(self, hge_ctx): @@ -418,7 +418,7 @@ class TestAddRemoteSchemaCompareRootQueryFields: def test_schema_check_arg_default_values_and_field_and_arg_types(self, hge_ctx): with open('queries/graphql_introspection/introspection.yaml') as f: query = yaml.safe_load(f) - introspect_hasura = check_query(hge_ctx, query) + introspect_hasura, _ = check_query(hge_ctx, query) resp = requests.post( self.remote, json=query['query'] diff --git a/server/tests-py/test_subscriptions.py b/server/tests-py/test_subscriptions.py index 46a7c478..e84d1eb2 100644 --- a/server/tests-py/test_subscriptions.py +++ b/server/tests-py/test_subscriptions.py @@ -3,7 +3,7 @@ import pytest import json import queue -import yaml +import ruamel.yaml as yaml from super_classes import GraphQLEngineTest diff --git a/server/tests-py/test_tests.py b/server/tests-py/test_tests.py new file mode 100755 index 00000000..3930dd1c --- /dev/null +++ b/server/tests-py/test_tests.py @@ -0,0 +1,160 @@ +#!/usr/bin/env python3 + +# This module is for tests that validate our tests or test framework, make sure +# tests are running correctly, or test our python test helpers. + +import pytest +from super_classes import DefaultTestSelectQueries +from validate import check_query_f, collapse_order_not_selset +from ruamel.yaml.comments import CommentedMap + +class TestTests1(DefaultTestSelectQueries): + """ + Test various things about our test framework code. Validate that tests work + as we expect. + """ + + # NOTE: We don't care about this for now, but should adapt this to test + # that xfail detection in code that handles `--accept` works correctly. + @pytest.mark.xfail(reason="expected") + def test_tests_xfail(self, request): + try: + marker = request.node.get_closest_marker("xfail") + print(marker) + if marker.name != 'xfail': + print("FAIL!") + return True # Force a test failure when xfail strict + except: + print("FAIL!") + return True # Force a test failure when xfail strict + assert 0, "Expected failure is expected" + + # Adapted arbitrarily from + # `TestGraphQLQueryBasic.test_select_query_author_pk()` using original yaml + # test case file that we later fixed. + @pytest.mark.xfail(reason="expected, validating test code") + def test_tests_detect_bad_ordering(self, hge_ctx): + """We can detect bad ordering of selection set""" + check_query_f(hge_ctx, 'test_tests/select_query_author_by_pkey_bad_ordering.yaml', 'http') + # + # E AssertionError: + # E expected: + # E data: + # E author_by_pk: + # E name: Author 1 + # E id: 1 + # E diff: (results differ only in their order of keys) + # E response: + # E data: + # E author_by_pk: + # E id: 1 + # E name: Author 1 + + + # Re-use setup and teardown from where we adapted this test case: + @classmethod + def dir(cls): + return 'queries/graphql_query/basic' + + +class TestTests2(DefaultTestSelectQueries): + """ + Test various things about our test framework code. Validate that tests work + as we expect. + """ + + # Test another bad ordering scenario, while we're here: + @pytest.mark.xfail(reason="expected, validating test code") + def test_tests_detect_bad_ordering(self, hge_ctx): + """We can detect bad ordering of selection set""" + check_query_f(hge_ctx, 'test_tests/user_can_query_jsonb_values_filter_bad_order.yaml', 'http') + # + # E AssertionError: + # E expected: + # E data: + # E jsonb_table: + # E - jsonb_col: + # E name: Hasura + # E age: 7 + # E id: 1 + # E response: + # E data: + # E jsonb_table: + # E - id: 1 + # E jsonb_col: + # E age: 7 + # E name: Hasura + # E diff: (results differ only in their order of keys) + + + # Unit test for good measure, to validate above and check our assumptions + # wrt comparisons of trees of ordered and unordered dicts and arrays: + def test_tests_dict_ordering_assumptions_and_helpers(self): + # fragment of yaml test file: + example_query = {"query": """ + query { + thing1 + jsonb_table{ + id + jsonb_col + } + thing2 + } + """ } + # We want to collapse any ordering we don't care about here + # (CommentedMap is ruamel.yaml's OrderedMap that also preserves + # format): + fully_ordered_result = \ + CommentedMap([('data', + CommentedMap([ + ('thing1', "thing1"), + ('jsonb_table', [ + CommentedMap([ + ('id', 1), + ('jsonb_col', CommentedMap([('age', 7), ('name', 'Hasura')]))]), + CommentedMap([ + ('id', 2), + ('jsonb_col', CommentedMap([('age', 8), ('name', 'Rawkz')]))]), + ]), + ('thing2', CommentedMap([("a",1), ("b",2), ("c",3)])), + ]))]) + + relevant_ordered_result = collapse_order_not_selset(fully_ordered_result, example_query) + + # We expect to have discarded ordering of leaves not in selset: + relevant_ordered_result_expected = \ + dict([('data', + CommentedMap([ + ('thing1', "thing1"), + ('jsonb_table', [ + CommentedMap([ + ('id', 1), + ('jsonb_col', dict([('age', 7), ('name', 'Hasura')]))]), + CommentedMap([ + ('id', 2), + ('jsonb_col', dict([('age', 8), ('name', 'Rawkz')]))]), + ]), + ('thing2', dict([("a",1), ("b",2), ("c",3)])), + ]))]) + + # NOTE: use str() to actually do a stong equality comparison, comparing + # types. Only works because str() on dict seems to have a canonical + # ordering. + assert str(relevant_ordered_result) == str(relevant_ordered_result_expected) + + # Demonstrate equality on different mixes of trees of ordered and unordered dicts: + assert CommentedMap([("a", "a"), ("b", "b")]) == dict([("b", "b"), ("a", "a")]) + assert CommentedMap([("a", "a"), ("b", "b")]) != CommentedMap([("b", "b"), ("a", "a")]) + assert dict([ ("x", CommentedMap([("a", "a"), ("b", CommentedMap([("b1", "b1"), ("b2", "b2")]))])), ("y","y"),]) == \ + CommentedMap([("y","y"), ("x", dict([("a", "a"), ("b", CommentedMap([("b1", "b1"), ("b2", "b2")]))])), ]) + + def test_tests_ordering_differences_correctly_ignored(self, hge_ctx): + """ + We don't care about ordering of stuff outside the selection set e.g. JSON fields. + """ + check_query_f(hge_ctx, 'test_tests/user_can_query_jsonb_values_filter_okay_orders.yaml', 'http') + + # Re-use setup and teardown from where we adapted this test case: + @classmethod + def dir(cls): + return 'queries/graphql_query/permissions' diff --git a/server/tests-py/test_tests/select_query_author_by_pkey_bad_ordering.yaml b/server/tests-py/test_tests/select_query_author_by_pkey_bad_ordering.yaml new file mode 100644 index 00000000..5adeb7f8 --- /dev/null +++ b/server/tests-py/test_tests/select_query_author_by_pkey_bad_ordering.yaml @@ -0,0 +1,17 @@ +description: select query on author with id = 1 +url: /v1/graphql +status: 200 +response: + data: + author_by_pk: + # Note: bad ordering + name: Author 1 + id: 1 +query: + query: | + query { + author_by_pk(id: 1){ + id + name + } + } diff --git a/server/tests-py/test_tests/user_can_query_jsonb_values_filter_bad_order.yaml b/server/tests-py/test_tests/user_can_query_jsonb_values_filter_bad_order.yaml new file mode 100644 index 00000000..79f8aa35 --- /dev/null +++ b/server/tests-py/test_tests/user_can_query_jsonb_values_filter_bad_order.yaml @@ -0,0 +1,21 @@ +description: User can query geometry values which satisfies filter in select permission + (order is incorrect, should fail) +url: /v1/graphql +status: 200 +headers: + X-Hasura-Role: user1 +response: + data: + jsonb_table: + - jsonb_col: + name: Hasura + age: 7 + id: 1 +query: + query: | + query { + jsonb_table{ + id + jsonb_col + } + } diff --git a/server/tests-py/test_tests/user_can_query_jsonb_values_filter_okay_orders.yaml b/server/tests-py/test_tests/user_can_query_jsonb_values_filter_okay_orders.yaml new file mode 100644 index 00000000..87c1f015 --- /dev/null +++ b/server/tests-py/test_tests/user_can_query_jsonb_values_filter_okay_orders.yaml @@ -0,0 +1,42 @@ +- description: User can query geometry values which satisfies filter in select permission (valid ordering alternative) + url: /v1/graphql + status: 200 + headers: + X-Hasura-Role: user1 + response: + data: + jsonb_table: + - id: 1 + jsonb_col: + name: Hasura + age: 7 + query: + query: | + query { + jsonb_table{ + id + jsonb_col + } + } +- description: User can query geometry values which satisfies filter in select permission (valid ordering alternative) + url: /v1/graphql + status: 200 + headers: + X-Hasura-Role: user1 + response: + data: + jsonb_table: + - id: 1 + jsonb_col: + # Note, order swapped; this is valid too (for now?): + age: 7 + name: Hasura + # Note: same query: + query: + query: | + query { + jsonb_table{ + id + jsonb_col + } + } diff --git a/server/tests-py/test_v1_queries.py b/server/tests-py/test_v1_queries.py index 43b82109..5bcabf94 100644 --- a/server/tests-py/test_v1_queries.py +++ b/server/tests-py/test_v1_queries.py @@ -1,4 +1,4 @@ -import yaml +import ruamel.yaml as yaml from validate import check_query_f from super_classes import DefaultTestSelectQueries, DefaultTestQueries, DefaultTestMutations diff --git a/server/tests-py/test_v1alpha1_endpoint.py b/server/tests-py/test_v1alpha1_endpoint.py index 52c756ba..347a795e 100644 --- a/server/tests-py/test_v1alpha1_endpoint.py +++ b/server/tests-py/test_v1alpha1_endpoint.py @@ -1,4 +1,4 @@ -import yaml +import ruamel.yaml as yaml import pytest #from validate import check_query, test_forbidden_when_admin_secret_reqd, test_forbidden_webhook from validate import check_query diff --git a/server/tests-py/test_validation.py b/server/tests-py/test_validation.py index 932f8469..2c4b1c92 100644 --- a/server/tests-py/test_validation.py +++ b/server/tests-py/test_validation.py @@ -1,5 +1,5 @@ import pytest -import yaml +import ruamel.yaml as yaml from validate import check_query_f from super_classes import GraphQLEngineTest diff --git a/server/tests-py/validate.py b/server/tests-py/validate.py index e5c4a09e..0e25fb77 100644 --- a/server/tests-py/validate.py +++ b/server/tests-py/validate.py @@ -1,13 +1,18 @@ #!/usr/bin/env python3 -import yaml +import pytest +import ruamel.yaml as yaml import json +import copy +import graphql import os import base64 +import json import jsondiff import jwt import random import time +import warnings from context import GQLWsClient @@ -116,6 +121,8 @@ def test_forbidden_webhook(hge_ctx, conf): }) +# Returns the response received and a bool indicating whether the test passed +# or not (this will always be True unless we are `--accepting`) def check_query(hge_ctx, conf, transport='http', add_auth=True): headers = {} if 'headers' in conf: @@ -215,17 +222,11 @@ def validate_gql_ws_q(hge_ctx, endpoint, query, headers, exp_http_response, retr else: assert resp['type'] == 'data', resp - exp_ws_response = exp_http_response - assert 'payload' in resp, resp - assert resp['payload'] == exp_ws_response, yaml.dump({ - 'response': resp['payload'], - 'expected': exp_ws_response, - 'diff': jsondiff.diff(exp_ws_response, resp['payload']) - }) resp_done = next(query_resp) assert resp_done['type'] == 'complete' - return resp['payload'] + + return assert_graphql_resp_expected(resp['payload'], exp_http_response, query) def validate_http_anyq(hge_ctx, url, query, headers, exp_code, exp_response): @@ -234,32 +235,147 @@ def validate_http_anyq(hge_ctx, url, query, headers, exp_code, exp_response): assert code == exp_code, resp print('http resp: ', resp) if exp_response: - assert json_ordered(resp) == json_ordered(exp_response), yaml.dump({ - 'response': resp, - 'expected': exp_response, - 'diff': jsondiff.diff(exp_response, resp) - }) - return resp + return assert_graphql_resp_expected(resp, exp_response, query) + else: + return resp, True + +# Check the actual graphql response is what we expected, also taking into +# consideration the ordering of keys that we expect to be preserved, based on +# 'query'. +# +# Returns 'resp' and a bool indicating whether the test passed or not (this +# will always be True unless we are `--accepting`) +def assert_graphql_resp_expected(resp_orig, exp_response_orig, query): + # Prepare actual and respected responses so comparison takes into + # consideration only the ordering that we care about: + resp = collapse_order_not_selset(resp_orig, query) + exp_response = collapse_order_not_selset(exp_response_orig, query) + matched = resp == exp_response + + if pytest.config.getoption("--accept"): + print('skipping assertion since we chose to --accept new output') + else: + assert matched, '\n' + yaml.dump({ + # Keep strict received order when displaying errors: + 'response': resp_orig, + 'expected': exp_response_orig, + 'diff': + (lambda diff: + "(results differ only in their order of keys)" if diff == {} else diff) + (stringify_keys(jsondiff.diff(exp_response, resp))) + }, Dumper=yaml.RoundTripDumper ) + return resp, matched # matched always True unless --accept + def check_query_f(hge_ctx, f, transport='http', add_auth=True): print("Test file: " + f) hge_ctx.may_skip_test_teardown = False print ("transport="+transport) - with open(f) as c: - conf = yaml.safe_load(c) + with open(f, 'r+') as c: + # For `--accept`: + should_write_back = False + + # ruamel RoundTripLoader will preserve order so that we can test the + # JSON ordering property conforms to YAML spec. + # It also lets us write back the yaml nicely when we --accept. + conf = yaml.load(c, yaml.RoundTripLoader) if isinstance(conf, list): - for sconf in conf: - check_query(hge_ctx, sconf, transport, add_auth) + for ix, sconf in enumerate(conf): + actual_resp, matched = check_query(hge_ctx, sconf, transport, add_auth) + if pytest.config.getoption("--accept") and not matched: + conf[ix]['response'] = actual_resp + should_write_back = True else: if conf['status'] != 200: hge_ctx.may_skip_test_teardown = True - check_query(hge_ctx, conf, transport, add_auth) + actual_resp, matched = check_query(hge_ctx, conf, transport, add_auth) + # If using `--accept` write the file back out with the new expected + # response set to the actual response we got: + if pytest.config.getoption("--accept") and not matched: + conf['response'] = actual_resp + should_write_back = True + + # TODO only write back when this test is not xfail. I'm stumped on how + # best to do this. Where the 'request' fixture comes into scope we can + # do : `request.node.get_closest_marker("xfail")` but don't want to + # require that everywhere... + if should_write_back: + warnings.warn( + "\nRecording formerly failing case as correct in: " + f + + "\n NOTE: if this case was marked 'xfail' this won't be correct!" + ) + c.seek(0) + c.write(yaml.dump(conf, Dumper=yaml.RoundTripDumper)) + c.truncate() -def json_ordered(obj): - if isinstance(obj, dict): - return sorted((k, json_ordered(v)) for k, v in obj.items()) - if isinstance(obj, list): - return list(json_ordered(x) for x in obj) +# Return a new dict that discards the object key ordering properties of +# 'result' where the key is not part of the selection set. This lets us compare +# expected and actual results properly with respect to the graphql spec's +# ordering requirements. +def collapse_order_not_selset(result_inp, query): + # Collapse to unordered dict recursively by roundtripping through json + def collapse(x): + return json.loads(json.dumps(x)) + + result = copy.deepcopy(result_inp) + try: + if 'query' in query: + gql_query_str = query['query'] + # We don't support multiple operations in the same query yet: + selset0 = graphql.parse(gql_query_str).definitions[0].selection_set + def go(result_node, selset): + for field in selset.selections: + fname = field.name.value + + # If field has no subfields then all its values can be recursively stripped of ordering. + # Also if it's an array for some reason (like in 'returning') TODO make this better + if field.selection_set is None or not isinstance(result_node[fname], (dict, list)): + result_node[fname] = collapse(result_node[fname]) + elif isinstance(result_node[fname], list): + for node in result_node[fname]: + go(node, field.selection_set) + else: + go(result_node[fname], field.selection_set) + + if 'data' in result: + go(result['data'], selset0) + # errors is unordered I guess + if 'errors' in result: + result['errors'] = collapse(result['errors']) + # and finally remove ordering at just the topmost level: + return dict(result) else: - return obj + # this isn't a graphql query, collapse ordering, I guess: + return collapse(result_inp) + + # Bail out here for any number of reasons. TODO improve me + except Exception as e: + print("Bailing out and collapsing all ordering, due to: ", e) + return collapse(result) + + +# Use this since jsondiff seems to produce object/dict structures that can't +# always be serialized to json. +# Copy-pasta from: https://stackoverflow.com/q/12734517/176841 +def stringify_keys(d): + """Convert a dict's keys to strings if they are not.""" + for key in d.keys(): + # check inner dict + if isinstance(d[key], dict): + value = stringify_keys(d[key]) + else: + value = d[key] + # convert nonstring to string if needed + if not isinstance(key, str): + try: + d[key.decode("utf-8")] = value + except Exception: + try: + d[repr(key)] = value + except Exception: + raise + + # delete old key + del d[key] + return d