misc improvements, test error conditions, remove redundant unit tests

Author: eli-bl

Diff for: end_to_end_tests/end_to_end_test_helpers.py

@@ -1,5 +1,6 @@
 import importlib
 import os
+import re
 import shutil
 from filecmp import cmpfiles, dircmp
 from pathlib import Path
@@ -92,7 +93,6 @@ def generate_client(
     if overwrite:
         args = [*args, "--overwrite"]
     generator_result = _run_command("generate", args, openapi_document, raise_on_error=raise_on_error)
-    print(generator_result.stdout)
     return GeneratedClientContext(
         full_output_path,
         generator_result,
@@ -107,21 +107,20 @@ def generate_client_from_inline_spec(
     filename_suffix: Optional[str] = None,
     config: str = "",
     base_module: str = "testapi_client",
-    add_openapi_info = True,
+    add_missing_sections = True,
     raise_on_error: bool = True,
 ) -> GeneratedClientContext:
     """Run the generator on a temporary file created with the specified contents.
     
     You can also optionally tell it to create a temporary config file.
     """
-    if add_openapi_info and not openapi_spec.lstrip().startswith("openapi:"):
-        openapi_spec += """
-openapi: "3.1.0"
-info:
-  title: "testapi"
-  description: "my test api"
-  version: "0.0.1"
-"""
+    if add_missing_sections:
+        if not re.search("^openapi:", openapi_spec, re.MULTILINE):
+            openapi_spec += "\nopenapi: '3.1.0'\n"
+        if not re.search("^info:", openapi_spec, re.MULTILINE):
+            openapi_spec += "\ninfo: {'title': 'testapi', 'description': 'my test api', 'version': '0.0.1'}\n"
+        if not re.search("^paths:", openapi_spec, re.MULTILINE):
+            openapi_spec += "\npaths: {}\n"
 
     output_path = tempfile.mkdtemp()
     file = tempfile.NamedTemporaryFile(suffix=filename_suffix, delete=False)
@@ -148,6 +147,43 @@ def generate_client_from_inline_spec(
     return generated_client
 
 
+def inline_spec_should_fail(
+    openapi_spec: str,
+    extra_args: List[str] = [],
+    filename_suffix: Optional[str] = None,
+    config: str = "",
+    add_missing_sections = True,
+) -> Result:
+    """Asserts that the generator could not process the spec.
+    
+    Returns the full output.
+    """
+    with generate_client_from_inline_spec(
+        openapi_spec, extra_args, filename_suffix, config, add_missing_sections, raise_on_error=False
+    ) as generated_client:
+        assert generated_client.generator_result.exit_code != 0
+        return generated_client.generator_result.stdout
+
+
+def inline_spec_should_cause_warnings(
+    openapi_spec: str,
+    extra_args: List[str] = [],
+    filename_suffix: Optional[str] = None,
+    config: str = "",
+    add_openapi_info = True,
+) -> str:
+    """Asserts that the generator is able to process the spec, but printed warnings.
+    
+    Returns the full output.
+    """
+    with generate_client_from_inline_spec(
+        openapi_spec, extra_args, filename_suffix, config, add_openapi_info, raise_on_error=True
+    ) as generated_client:
+        assert generated_client.generator_result.exit_code == 0
+        assert "Warning(s) encountered while generating" in generated_client.generator_result.stdout
+        return generated_client.generator_result.stdout
+
+
 def with_generated_client_fixture(
     openapi_spec: str,
     name: str="generated_client",
@@ -197,7 +233,31 @@ def _func(self, generated_client):
     return _decorator
 
 
-def assert_model_decode_encode(model_class: Any, json_data: dict, expected_instance: Any):
+def with_generated_code_imports(*import_paths: str):
+    def _decorator(cls):
+        decorated = cls
+        for import_path in import_paths:
+            decorated = with_generated_code_import(import_path)(decorated)
+        return decorated
+
+    return _decorator
+
+
+def assert_model_decode_encode(model_class: Any, json_data: dict, expected_instance: Any) -> None:
     instance = model_class.from_dict(json_data)
     assert instance == expected_instance
     assert instance.to_dict() == json_data
+
+
+def assert_bad_schema_warning(output: str, schema_name: str, expected_message_str) -> None:
+    bad_schema_regex = "Unable to (parse|process) schema"
+    expected_start_regex = f"{bad_schema_regex} /components/schemas/{re.escape(schema_name)}:?\n"
+    if not (match := re.search(expected_start_regex, output)):
+        # this assert is to get better failure output
+        assert False, f"Did not find '{expected_start_regex}' in output: {output}"
+    output = output[match.end():]
+    # The amount of other information in between that message and the warning detail can vary
+    # depending on the error, so just make sure we're not picking up output from a different schema
+    if (next_match := re.search(bad_schema_regex, output)):
+        output = output[0:next_match.start()]
+    assert expected_message_str in output

Diff for: end_to_end_tests/generated_code_live_tests/README.md

@@ -5,14 +5,14 @@ These are end-to-end tests which run the code generator command, but unlike the
 Each test class follows this pattern:
 
 - Use the decorator `@with_generated_client_fixture`, providing an inline API spec (JSON or YAML) that contains whatever schemas/paths/etc. are relevant to this test class. 
-  - The spec can omit the `openapi:` and `info:` blocks, unless those are relevant to the test.
+  - The spec can omit the `openapi:`, `info:`, and `paths:`, blocks, unless those are relevant to the test.
   - The decorator creates a temporary file for the inline spec and a temporary directory for the generated code, and runs the client generator.
   - It creates a `GeneratedClientContext` object (defined in `end_to_end_test_helpers.py`) to keep track of things like the location of the generated code and the output of the generator command.
   - This object is injected into the test class as a fixture called `generated_client`, although most tests will not need to reference the fixture directly.
   - `sys.path` is temporarily changed, for the scope of this test class, to allow imports from the generated code.
-- Use the decorator `@with_generated_code_import` to make classes or functions from the generated code available to the tests.
-  - `@with_generated_code_import(".models.MyModel")` would execute `from [client package name].models import MyModel` and inject the imported object into the test class as a fixture called `MyModel`.
-  - `@with_generated_code_import(".models.MyModel", alias="model1")` would do the same thing, but the fixture would be named `model1`.
+- Use the decorator `@with_generated_code_imports` or `@with_generated_code_import` to make classes or functions from the generated code available to the tests.
+  - `@with_generated_code_imports(".models.MyModel1", ".models.MyModel2)` would execute `from [package name].models import MyModel1, MyModel2` and inject the imported classes into the test class as fixtures called `MyModel1` and `MyModel2`.
+  - `@with_generated_code_import(".api.my_operation.sync", alias="endpoint_method")` would execute `from [package name].api.my_operation import sync`, but the fixture would be named `endpoint_method`.
   - After the test class finishes, these imports are discarded.
 
 Example:
@@ -21,7 +21,6 @@ Example:
 
 @with_generated_client_fixture(
 """
-paths: {}
 components:
   schemas:
     MyModel:

Diff for: end_to_end_tests/generated_code_live_tests/test_defaults.py

@@ -0,0 +1,98 @@
+
+import datetime
+import uuid
+import pytest
+from end_to_end_tests.end_to_end_test_helpers import (
+    assert_bad_schema_warning,
+    assert_model_decode_encode,
+    inline_spec_should_cause_warnings,
+    with_generated_client_fixture,
+    with_generated_code_imports,
+)
+
+
+@with_generated_client_fixture(
+"""
+components:
+  schemas:
+    MyModel:
+      type: object
+      properties:
+        booleanProp: {"type": "boolean", "default": true}
+        stringProp: {"type": "string", "default": "a"}
+        numberProp: {"type": "number", "default": 1.5}
+        intProp: {"type": "integer", "default": 2}
+        noneProp: {"type": "null", "default": null}
+        anyPropWithString: {"default": "b"}
+        anyPropWithInt: {"default": 3}
+        booleanWithStringTrue1: {"type": "boolean", "default": "True"}
+        booleanWithStringTrue2: {"type": "boolean", "default": "true"}
+        booleanWithStringFalse1: {"type": "boolean", "default": "False"}
+        booleanWithStringFalse2: {"type": "boolean", "default": "false"}
+        intWithStringValue: {"type": "integer", "default": "4"}
+        numberWithIntValue: {"type": "number", "default": 5}
+        numberWithStringValue: {"type": "number", "default": "5.5"}
+        noneWithStringValue: {"type": "null", "default": "None"}
+""")
+@with_generated_code_imports(".models.MyModel")
+class TestDefaultValues:
+    def test_defaults_in_initializer(self, MyModel, generated_client):
+        instance = MyModel()
+        assert instance == MyModel(
+            boolean_prop=True,
+            string_prop="a",
+            number_prop=1.5,
+            int_prop=2,
+            any_prop_with_string="b",
+            any_prop_with_int=3,
+            boolean_with_string_true_1=True,
+            boolean_with_string_true_2=True,
+            boolean_with_string_false_1=False,
+            boolean_with_string_false_2=False,
+            int_with_string_value=4,
+            number_with_int_value=5,
+            number_with_string_value=5.5,
+        )
+        # Note, currently the default for a None property does not work as expected--
+        # the initializer will default it to UNSET rather than None.
+
+
+class TestInvalidDefaultValues:
+    @pytest.fixture(scope="class")
+    def warnings(self):
+        return inline_spec_should_cause_warnings(
+"""
+components:
+  schemas:
+    WithBadBoolean:
+      properties:
+        badBoolean: {"type": "boolean", "default": "not a boolean"}
+    WithBadIntAsString:
+      properties:
+        badInt: {"type": "integer", "default": "not an int"}
+    WithBadIntAsOther:
+      properties:
+        badInt: {"type": "integer", "default": true}
+    WithBadFloatAsString:
+      properties:
+        badInt: {"type": "number", "default": "not a number"}
+    WithBadFloatAsOther:
+      properties:
+        badInt: {"type": "number", "default": true}
+"""
+        )
+
+    def test_bad_boolean(self, warnings):
+        assert_bad_schema_warning(warnings, "WithBadBoolean", "Invalid boolean value")
+
+    def test_bad_int_as_string(self, warnings):
+        assert_bad_schema_warning(warnings, "WithBadIntAsString", "Invalid int value")
+
+    def test_bad_int_as_other(self, warnings):
+        assert_bad_schema_warning(warnings, "WithBadIntAsOther", "Invalid int value")
+
+    def test_bad_float_as_string(self, warnings):
+        assert_bad_schema_warning(warnings, "WithBadFloatAsString", "Invalid float value")
+
+    def test_bad_float_as_other(self, warnings):
+        assert_bad_schema_warning(warnings, "WithBadFloatAsOther", "Cannot convert True to a float")

Diff for: end_to_end_tests/generated_code_live_tests/test_docstrings.py

@@ -18,7 +18,6 @@ def get_section(self, header_line: str) -> List[str]:
 
 @with_generated_client_fixture(
 """
-paths: {}
 components:
   schemas:
     MyModel:

Diff for: end_to_end_tests/generated_code_live_tests/test_enums.py renamed to end_to_end_tests/generated_code_live_tests/test_enum_and_const.py

@@ -1,15 +1,18 @@
 
+from typing import Any
 import pytest
 from end_to_end_tests.end_to_end_test_helpers import (
+    assert_bad_schema_warning,
     assert_model_decode_encode,
+    inline_spec_should_cause_warnings,
     with_generated_code_import,
     with_generated_client_fixture,
+    with_generated_code_imports,
 )
 
 
 @with_generated_client_fixture(
 """
-paths: {}
 components:
   schemas:
     MyEnum:
@@ -27,9 +30,7 @@
             - {"$ref": "#/components/schemas/MyEnum"}
             - type: "null"
 """)
-@with_generated_code_import(".models.MyEnum")
-@with_generated_code_import(".models.MyIntEnum")
-@with_generated_code_import(".models.MyModel")
+@with_generated_code_imports(".models.MyEnum", ".models.MyIntEnum", ".models.MyModel")
 class TestEnumClasses:
     def test_enum_classes(self, MyEnum, MyIntEnum):
         assert MyEnum.A == MyEnum("a")
@@ -72,7 +73,6 @@ def test_invalid_values(self, MyModel):
 
 @with_generated_client_fixture(
 """
-paths: {}
 components:
   schemas:
     MyEnum:
@@ -130,24 +130,75 @@ def test_invalid_values(self, MyModel):
 
 @with_generated_client_fixture(
 """
-paths: {}
 components:
   schemas:
     MyModel:
       properties:
         mustBeErnest:
           const: Ernest
+        mustBeThirty:
+          const: 30
 """,
 )
 @with_generated_code_import(".models.MyModel")
 class TestConst:
-    def test_valid_value(self, MyModel):
+    def test_valid_string(self, MyModel):
         assert_model_decode_encode(
             MyModel,
             {"mustBeErnest": "Ernest"},
             MyModel(must_be_ernest="Ernest"),
         )
 
-    def test_invalid_value(self, MyModel):
+    def test_valid_int(self, MyModel):
+        assert_model_decode_encode(
+            MyModel,
+            {"mustBeThirty": 30},
+            MyModel(must_be_thirty=30),
+        )
+
+    def test_invalid_string(self, MyModel):
         with pytest.raises(ValueError):
             MyModel.from_dict({"mustBeErnest": "Jack"})
+
+    def test_invalid_int(self, MyModel):
+        with pytest.raises(ValueError):
+            MyModel.from_dict({"mustBeThirty": 29})
+
+
+class TestEnumAndConstInvalidSchemas:
+    @pytest.fixture(scope="class")
+    def warnings(self):
+        return inline_spec_should_cause_warnings(
+"""
+components:
+  schemas:
+    WithBadDefaultValue:
+      enum: ["A"]
+      default: "B"
+    WithBadDefaultType:
+      enum: ["A"]
+      default: 123
+    WithMixedTypes:
+      enum: ["A", 1]
+    WithUnsupportedType:
+      enum: [1.4, 1.5]
+    DefaultNotMatchingConst:
+      const: "aaa"
+      default: "bbb"
+"""
+        )
+
+    def test_enum_bad_default_value(self, warnings):
+        assert_bad_schema_warning(warnings, "WithBadDefaultValue", "Value B is not valid")
+
+    def test_enum_bad_default_type(self, warnings):
+        assert_bad_schema_warning(warnings, "WithBadDefaultType", "Cannot convert 123 to enum")
+
+    def test_enum_mixed_types(self, warnings):
+        assert_bad_schema_warning(warnings, "WithMixedTypes", "Enum values must all be the same type")
+
+    def test_enum_unsupported_type(self, warnings):
+        assert_bad_schema_warning(warnings, "WithUnsupportedType", "Unsupported enum type")
+
+    def test_const_default_not_matching(self, warnings):
+        assert_bad_schema_warning(warnings, "DefaultNotMatchingConst", "Invalid value for const")