From 5677446ff3fc8ad162b40207702e750aaa56bf4f Mon Sep 17 00:00:00 2001 From: Samir Talwar Date: Wed, 12 Apr 2023 18:24:47 +0200 Subject: [PATCH] server/tests-py: Run test_websocket_init_cookie.py in parallel. This requires rewriting the test class to split it into 3, each specifying the correct environment variables for HGE. It would be lovely to use parameterization rather than subclassing, but that doesn't work because of `hge_fixture_env`, which creates a "soft" dependency between the environment variables and `hge_server`. Parameterizing the former *should* force the latter to be recreated for each new set of environment variables, but `hge_server` isn't actually aware there's a dependency. See `TestParameterizedFixtures` in test_tests.py for more information. [NDAT-539]: https://hasurahq.atlassian.net/browse/NDAT-539?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ PR-URL: https://github.com/hasura/graphql-engine-mono/pull/8747 GitOrigin-RevId: 878b2fc20f39f962a67cd950046a99c283cfc6fc --- .circleci/server-test-names.txt | 6 +- server/tests-py/conftest.py | 8 --- server/tests-py/context.py | 2 - server/tests-py/test_websocket_init_cookie.py | 66 +++++++++++-------- 4 files changed, 40 insertions(+), 42 deletions(-) diff --git a/.circleci/server-test-names.txt b/.circleci/server-test-names.txt index 124868b1edf..e8db8766831 100644 --- a/.circleci/server-test-names.txt +++ b/.circleci/server-test-names.txt @@ -32,9 +32,9 @@ jwt-cookie jwt-cookie-unauthorized-role # cors-domains # auth-webhook-cookie -ws-init-cookie-read-cors-enabled -ws-init-cookie-noread -ws-init-cookie-read-cors-disabled +# ws-init-cookie-read-cors-enabled +# ws-init-cookie-noread +# ws-init-cookie-read-cors-disabled # ws-graphql-api-disabled # ws-metadata-api-disabled # remote-schema-permissions diff --git a/server/tests-py/conftest.py b/server/tests-py/conftest.py index 5fe92dec273..1acbc857831 100644 --- a/server/tests-py/conftest.py +++ b/server/tests-py/conftest.py @@ -3,7 +3,6 @@ import http.server import inspect import os import pytest -import re import socket import sqlalchemy import sys @@ -53,13 +52,6 @@ def pytest_addoption(parser): "--hge-jwt-conf", metavar="HGE_JWT_CONF", help="The JWT conf", required=False ) - parser.addoption( - "--test-ws-init-cookie", - metavar="read|noread", - required=False, - help="Run testcases for testing cookie sending over websockets" - ) - parser.addoption( "--test-hge-scale-url", metavar="", diff --git a/server/tests-py/context.py b/server/tests-py/context.py index 530d4db2b4c..4a9ae5064ad 100644 --- a/server/tests-py/context.py +++ b/server/tests-py/context.py @@ -856,8 +856,6 @@ class HGECtx: self.engine = sqlalchemy.create_engine(self.metadata_schema_url) self.meta = sqlalchemy.schema.MetaData() - self.ws_read_cookie = config.getoption('--test-ws-init-cookie') - self.hge_scale_url = config.getoption('--test-hge-scale-url') self.avoid_err_msg_checks = config.getoption('--avoid-error-message-checks') self.pro_tests = config.getoption('--pro-tests') diff --git a/server/tests-py/test_websocket_init_cookie.py b/server/tests-py/test_websocket_init_cookie.py index db01610ac06..0ab1ff5f7d8 100644 --- a/server/tests-py/test_websocket_init_cookie.py +++ b/server/tests-py/test_websocket_init_cookie.py @@ -3,17 +3,20 @@ from urllib.parse import urlparse import websocket import pytest -from context import PytestConf - -if not PytestConf.config.getoption("--test-ws-init-cookie"): - pytest.skip("--test-ws-init-cookie flag is missing, skipping tests", allow_module_level=True) def url(hge_ctx): ws_url = urlparse(hge_ctx.hge_url)._replace(scheme='ws', path='/v1alpha1/graphql') return ws_url.geturl() -@pytest.mark.usefixtures('auth_hook') -class TestWebsocketInitCookie(): +# The tests below would ideally use parameterization rather than subclassing, +# but that doesn't work because of `hge_fixture_env`, which creates a "soft" +# dependency between the environment variables and `hge_server`. Parameterizing +# the former *should* force the latter to be recreated for each new set of +# environment variables, but `hge_server` isn't actually aware there's a +# dependency. See `TestParameterizedFixtures` in test_tests.py for more +# information. + +class AbstractTestWebsocketInitCookie: """ test if cookie is sent when initing the websocket connection, is our auth webhook receiving the cookie @@ -43,9 +46,7 @@ class TestWebsocketInitCookie(): ws.send(json.dumps(payload)) return ws - def test_websocket_init_cookie_used(self, hge_ctx): - if hge_ctx.ws_read_cookie == 'noread': - pytest.skip('cookie is not to be read') + def _test_received_data(self, hge_ctx): ws = self._send_query(hge_ctx) it = 0 while True: @@ -55,38 +56,45 @@ class TestWebsocketInitCookie(): assert 'person' in frame['payload']['data'] break elif it == 10: - print('max try over') - assert False - break + assert False, f'max try over: {raw}' elif frame['type'] == 'connection_error' or frame['type'] == 'error': - print(frame) - assert False - break + assert False, f'connection error: {raw}' it = it + 1 - def test_websocket_init_cookie_not_used(self, hge_ctx): - if hge_ctx.ws_read_cookie == 'read': - pytest.skip('cookie is read') - + def _test_received_connection_error(self, hge_ctx): ws = self._send_query(hge_ctx) it = 0 while True: raw = ws.recv() frame = json.loads(raw) if frame['type'] == 'data': - print('got data') - assert False - break + assert False, f'got data: {raw}' elif it == 10: - print('max try over') - assert False - break + assert False, f'max try over: {raw}' elif frame['type'] == 'connection_error': - print(frame) assert frame['payload'] == 'Authentication hook unauthorized this request' break elif frame['type'] == 'error': - print(frame) - assert False - break + assert False, f'error: {raw}' it = it + 1 + +@pytest.mark.admin_secret +@pytest.mark.usefixtures('auth_hook') +class TestWebsocketInitCookieReadWithCorsEnabled(AbstractTestWebsocketInitCookie): + def test_websocket_init_cookie_used(self, hge_ctx): + self._test_received_data(hge_ctx) + +@pytest.mark.admin_secret +@pytest.mark.usefixtures('auth_hook') +@pytest.mark.hge_env('HASURA_GRAPHQL_DISABLE_CORS', 'true') +class TestWebsocketInitCookieNotReadWithCorsDisabled(AbstractTestWebsocketInitCookie): + def test_websocket_init_cookie_not_used(self, hge_ctx): + self._test_received_connection_error(hge_ctx) + +@pytest.mark.admin_secret +@pytest.mark.usefixtures('auth_hook') +@pytest.mark.hge_env('HASURA_GRAPHQL_DISABLE_CORS', 'true') +@pytest.mark.hge_env('HASURA_GRAPHQL_WS_READ_COOKIE', 'true') +class TestWebsocketInitCookieReadWithCorsDisabledWhenRequired(AbstractTestWebsocketInitCookie): + def test_websocket_init_cookie_not_used(self, hge_ctx): + self._test_received_data(hge_ctx)