From c91c106cfb6477f278452cfb4d2e2797b8ee831b Mon Sep 17 00:00:00 2001 From: orientor Date: Sat, 25 Jul 2020 20:54:21 +0530 Subject: [PATCH] openapi_py: Change condition for invalid requests. Change the condition for allowing failed validation to the condition that `if the test fails, response status code begins with 4`. Also add `intentionally_undocumented` argument in `validate_request` for allowing passing of tests which return `200` responses but fail validation due to some intentionally undocumented feature in OpenAPI specification. --- zerver/openapi/openapi.py | 21 ++++++++++++--------- zerver/tests/test_openapi.py | 19 +++++++++++-------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/zerver/openapi/openapi.py b/zerver/openapi/openapi.py index 31e7b5a7aa..baa2b2caab 100644 --- a/zerver/openapi/openapi.py +++ b/zerver/openapi/openapi.py @@ -306,7 +306,7 @@ SKIP_JSON = {'/fetch_api_key:post'} def validate_request(url: str, method: str, data: Dict[str, Any], http_headers: Dict[str, Any], json_url: bool, - uses_invalid_parameters: bool) -> None: + status_code: str, intentionally_undocumented: bool=False) -> None: # Some JSON endpoints have different parameters compared to # their `/api/v1` counterparts. if json_url and url + ':' + method in SKIP_JSON: @@ -322,12 +322,15 @@ def validate_request(url: str, method: str, data: Dict[str, Any], mock_request = MockRequest('http://localhost:9991/', method, '/api/v1' + url, headers=http_headers, args=data) result = openapi_spec.core_validator().validate(mock_request) - - if uses_invalid_parameters: - # This assertion helps us avoid unnecessary use of the - # uses_invalid_parameters argument. - assert(len(result.errors) != 0) - return + if len(result.errors) != 0: + # Requests that do not validate against the OpenAPI spec must either: + # * Have returned a 400 (bad request) error + # * Have returned a 200 (success) with this request marked as intentionally + # undocumented behavior. + if status_code.startswith('4'): + return + if status_code.startswith('2') and intentionally_undocumented: + return # If no errors are raised, then validation is successful if len(result.errors) == 0: @@ -341,8 +344,8 @@ with the parameters passed in this HTTP request. Consider: * Updating the OpenAPI schema defined in zerver/openapi/zulip.yaml * Adjusting the test to pass valid parameters. If the test - deliberately passes invalid parameters, you need to pass - `uses_invalid_parameters=True` to self.client_{method.lower()} or + fails due to intentionally_undocumented features, you need to pass + `intentionally_undocumented=True` to self.client_{method.lower()} or self.api_{method.lower()} to document your intent. See https://zulip.readthedocs.io/en/latest/documentation/api.html for help. diff --git a/zerver/tests/test_openapi.py b/zerver/tests/test_openapi.py index db80e3eabb..28ec1d15de 100644 --- a/zerver/tests/test_openapi.py +++ b/zerver/tests/test_openapi.py @@ -1078,12 +1078,15 @@ class OpenAPIRequestValidatorTest(ZulipTestCase): """ # `/users/me/subscriptions` doesn't require any parameters validate_request('/users/me/subscriptions', 'get', {}, {}, False, - uses_invalid_parameters=False) - with self.assertRaises(AssertionError): - validate_request('/users/me/subscriptions', 'get', {}, {}, - False, uses_invalid_parameters=True) - validate_request('/dev_fetch_api_key', 'post', {}, {}, False, - uses_invalid_parameters=True) + '200') with self.assertRaises(SchemaError): - validate_request('/dev_fetch_api_key', 'post', {}, {}, - False, uses_invalid_parameters=False) + # `/messages` POST does not work on an empty response + validate_request('/messages', 'post', {}, {}, + False, '200') + # 400 responses are allowed to fail validation. + validate_request('/messages', 'post', {}, {}, + False, '400') + # `intentionally_undocumented` allows validation errors on + # 200 responses. + validate_request('/dev_fetch_api_key', 'post', {}, {}, + False, '200', intentionally_undocumented=True)