restore some missing test coverage

Author: eli-bl

Diff for: .changeset/live_tests.md

@@ -2,8 +2,13 @@
 default: minor
 ---
 
-# New category of end-to-end tests
+# New categories of end-to-end tests
 
-There is a new set of tests that generate client code from an API document and then actually import and execute that code. See [`end_to_end_tests/generated_code_live_tests`](./end_to_end_tests/generated_code_live_tests) for more details.
+Automated tests have been extended to include two new types of tests:
+
+1. Happy-path tests that run the generator from an inline API document and then actually import and execute the generated code. See [`end_to_end_tests/generated_code_live_tests`](./end_to_end_tests/generated_code_live_tests).
+2. Warning/error condition tests that run the generator from an inline API document that contains something invalid, and make assertions about the generator's output.
+
+These provide more efficient and granular test coverage than the "golden record"-based end-to-end tests, and also replace some tests that were previously being done against low-level implementation details in `tests/unit`.
 
 This does not affect any runtime functionality of openapi-python-client.

Diff for: CONTRIBUTING.md

@@ -77,11 +77,13 @@ The tests run the generator against a small API spec (defined inline for each te
 
 See [`end_to_end_tests/generated_code_live_tests`](./end_to_end_tests/generated_code_live_tests).
 
-#### Unit tests
+#### Other unit tests
 
-> **NOTE**: Several older-style unit tests using mocks exist in this project. These should be phased out rather than updated, as the tests are brittle and difficult to maintain. Only error cases should be tests with unit tests going forward.
+These include:
 
-In some cases, we need to test things which cannot be generated—like validating that errors are caught and handled correctly. These should be tested via unit tests in the `tests` directory, using the `pytest` framework.
+* Regular unit tests of basic pieces of fairly self-contained low-level functionality, such as helper functions. These are implemented in the `tests/unit` directory, using the `pytest` framework.
+* End-to-end tests of invalid spec conditions, where we run the generator against a small spec with some problem, and expect it to print warnings/errors rather than generating code. These are implemented in `end_to_end_tests/generator_errors_and_warnings`.
+* Older-style unit tests of low-level functions like `property_from_data` that have complex behavior. These are brittle and difficult to maintain, and should not be used going forward. Instead, use either unit tests of generated code (to test happy paths), or end-to-end tests of invalid spec conditions (to test for warnings/errors), as described above.
 
 ### Creating a Pull Request
 

Diff for: end_to_end_tests/end_to_end_test_helpers.py

@@ -78,18 +78,17 @@ def generate_client(
     extra_args: List[str] = [],
     output_path: str = "my-test-api-client",
     base_module: str = "my_test_api_client",
+    specify_output_path_explicitly: bool = True,
     overwrite: bool = True,
     raise_on_error: bool = True,
 ) -> GeneratedClientContext:
     """Run the generator and return a GeneratedClientContext for accessing the generated code."""
     full_output_path = Path.cwd() / output_path
     if not overwrite:
         shutil.rmtree(full_output_path, ignore_errors=True)
-    args = [
-        *extra_args,
-        "--output-path",
-        str(full_output_path),
-    ]
+    args = extra_args
+    if specify_output_path_explicitly:
+        args = [*args, "--output-path", str(full_output_path)]
     if overwrite:
         args = [*args, "--overwrite"]
     generator_result = _run_command("generate", args, openapi_document, raise_on_error=raise_on_error)
@@ -159,7 +158,7 @@ def inline_spec_should_fail(
     Returns the command result, which could include stdout data or an exception.
     """
     with generate_client_from_inline_spec(
-        openapi_spec, extra_args, filename_suffix, config, add_missing_sections, raise_on_error=False
+        openapi_spec, extra_args, filename_suffix, config, add_missing_sections=add_missing_sections, raise_on_error=False
     ) as generated_client:
         assert generated_client.generator_result.exit_code != 0
         return generated_client.generator_result
@@ -170,14 +169,14 @@ def inline_spec_should_cause_warnings(
     extra_args: List[str] = [],
     filename_suffix: Optional[str] = None,
     config: str = "",
-    add_openapi_info = True,
+    add_missing_sections = 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
+        openapi_spec, extra_args, filename_suffix, config, add_missing_sections=add_missing_sections, 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
@@ -250,6 +249,10 @@ def assert_model_decode_encode(model_class: Any, json_data: dict, expected_insta
     assert instance.to_dict() == json_data
 
 
+def assert_model_property_type_hint(model_class: Any, name: str, expected_type_hint: Any) -> None:
+    assert model_class.__annotations__[name] == expected_type_hint
+
+
 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"

Diff for: end_to_end_tests/generated_code_live_tests/README.md

@@ -18,7 +18,6 @@ Each test class follows this pattern:
 Example:
 
 ```python
-
 @with_generated_client_fixture(
 """
 components:

Diff for: end_to_end_tests/generated_code_live_tests/test_arrays.py

@@ -0,0 +1,152 @@
+import datetime
+from typing import Any, ForwardRef, List, Union
+import uuid
+import pytest
+from end_to_end_tests.end_to_end_test_helpers import (
+    assert_model_decode_encode,
+    assert_model_property_type_hint,
+    with_generated_client_fixture,
+    with_generated_code_imports,
+)
+
+
+@with_generated_client_fixture(
+"""
+components:
+  schemas:
+    SimpleObject:
+      type: object
+      properties:
+        name: {"type": "string"}
+    ModelWithArrayOfAny:
+      properties:
+        arrayProp:
+          type: array
+          items: {}
+    ModelWithArrayOfInts:
+      properties:
+        arrayProp:
+          type: array
+          items: {"type": "integer"}
+    ModelWithArrayOfObjects:
+      properties:
+        arrayProp:
+          type: array
+          items: {"$ref": "#/components/schemas/SimpleObject"}
+""")
+@with_generated_code_imports(
+    ".models.ModelWithArrayOfAny",
+    ".models.ModelWithArrayOfInts",
+    ".models.ModelWithArrayOfObjects",
+    ".models.SimpleObject",
+    ".types.Unset",
+)
+class TestArraySchemas:
+    def test_array_of_any(self, ModelWithArrayOfAny):
+        assert_model_decode_encode(
+            ModelWithArrayOfAny,
+            {"arrayProp": ["a", 1]},
+            ModelWithArrayOfAny(array_prop=["a", 1]),
+        )
+
+    def test_array_of_int(self, ModelWithArrayOfInts):
+        assert_model_decode_encode(
+            ModelWithArrayOfInts,
+            {"arrayProp": [1, 2]},
+            ModelWithArrayOfInts(array_prop=[1, 2]),
+        )
+        # Note, currently arrays of simple types are not validated, so the following assertion would fail:
+        # with pytest.raises(TypeError):
+        #    ModelWithArrayOfInt.from_dict({"arrayProp": [1, "a"]})
+
+    def test_array_of_object(self, ModelWithArrayOfObjects, SimpleObject):
+        assert_model_decode_encode(
+            ModelWithArrayOfObjects,
+            {"arrayProp": [{"name": "a"}, {"name": "b"}]},
+            ModelWithArrayOfObjects(array_prop=[SimpleObject(name="a"), SimpleObject(name="b")]),
+        )
+
+    def test_type_hints(self, ModelWithArrayOfAny, ModelWithArrayOfInts, ModelWithArrayOfObjects, Unset):
+        assert_model_property_type_hint(ModelWithArrayOfAny, "array_prop", Union[List[Any], Unset])
+        assert_model_property_type_hint(ModelWithArrayOfInts, "array_prop", Union[List[int], Unset])
+        assert_model_property_type_hint(ModelWithArrayOfObjects, "array_prop", Union[List[ForwardRef("SimpleObject")], Unset])
+
+
+@with_generated_client_fixture(
+"""
+components:
+  schemas:
+    SimpleObject:
+      type: object
+      properties:
+        name: {"type": "string"}
+    ModelWithSinglePrefixItem:
+      type: object
+      properties:
+        arrayProp:
+          type: array
+          prefixItems:
+            - type: string
+    ModelWithPrefixItems:
+      type: object
+      properties:
+        arrayProp:
+          type: array
+          prefixItems:
+            - $ref: "#/components/schemas/SimpleObject"
+            - type: string
+    ModelWithMixedItems:
+      type: object
+      properties:
+        arrayProp:
+          type: array
+          prefixItems:
+            - $ref: "#/components/schemas/SimpleObject"
+          items:
+            type: string
+""")
+@with_generated_code_imports(
+    ".models.ModelWithSinglePrefixItem",
+    ".models.ModelWithPrefixItems",
+    ".models.ModelWithMixedItems",
+    ".models.SimpleObject",
+    ".types.Unset",
+)
+class TestArraysWithPrefixItems:
+    def test_single_prefix_item(self, ModelWithSinglePrefixItem):
+        assert_model_decode_encode(
+            ModelWithSinglePrefixItem,
+            {"arrayProp": ["a"]},
+            ModelWithSinglePrefixItem(array_prop=["a"]),
+        )
+
+    def test_prefix_items(self, ModelWithPrefixItems, SimpleObject):
+        assert_model_decode_encode(
+            ModelWithPrefixItems,
+            {"arrayProp": [{"name": "a"}, "b"]},
+            ModelWithPrefixItems(array_prop=[SimpleObject(name="a"), "b"]),
+        )
+
+    def test_prefix_items_and_regular_items(self, ModelWithMixedItems, SimpleObject):
+        assert_model_decode_encode(
+            ModelWithMixedItems,
+            {"arrayProp": [{"name": "a"}, "b"]},
+            ModelWithMixedItems(array_prop=[SimpleObject(name="a"), "b"]),
+        )
+
+    def test_type_hints(self, ModelWithSinglePrefixItem, ModelWithPrefixItems, ModelWithMixedItems, Unset):
+        assert_model_property_type_hint(ModelWithSinglePrefixItem, "array_prop", Union[List[str], Unset])
+        assert_model_property_type_hint(
+            ModelWithPrefixItems,
+            "array_prop",
+            Union[List[Union[ForwardRef("SimpleObject"), str]], Unset],
+        )
+        assert_model_property_type_hint(
+            ModelWithMixedItems,
+            "array_prop",
+            Union[List[Union[ForwardRef("SimpleObject"), str]], Unset],
+        )
+        # Note, this test is asserting the current behavior which, due to limitations of the implementation
+        # (see: https://github.com/openapi-generators/openapi-python-client/pull/1130), is not really doing
+        # tuple type validation-- the ordering of prefixItems is ignored, and instead all of the types are
+        # simply treated as a union.

Diff for: end_to_end_tests/generated_code_live_tests/test_defaults.py

@@ -1,11 +1,6 @@
-
 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,
 )
@@ -36,6 +31,12 @@
         numberWithStringValue: {"type": "number", "default": "5.5"}
         stringWithNumberValue: {"type": "string", "default": 6}
         stringConst: {"type": "string", "const": "always", "default": "always"}
+        unionWithValidDefaultForType1:
+          anyOf: [{"type": "boolean"}, {"type": "integer"}]
+          default: true
+        unionWithValidDefaultForType2:
+          anyOf: [{"type": "boolean"}, {"type": "integer"}]
+          default: 3
 """)
 @with_generated_code_imports(".models.MyModel")
 class TestSimpleDefaults:
@@ -62,6 +63,8 @@ def test_defaults_in_initializer(self, MyModel):
             number_with_string_value=5.5,
             string_with_number_value="6",
             string_const="always",
+            union_with_valid_default_for_type_1=True,
+            union_with_valid_default_for_type_2=3,
         )
 
 
@@ -88,70 +91,23 @@ def test_enum_default(self, MyEnum, MyModel):
         assert MyModel().enum_prop == MyEnum.A
 
 
-class TestInvalidDefaultValues:
-    @pytest.fixture(scope="class")
-    def warnings(self):
-        return inline_spec_should_cause_warnings(
+@with_generated_client_fixture(
 """
 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}
-    WithBadDateAsString:
-      properties:
-        badDate: {"type": "string", "format": "date", "default": "xxx"}
-    WithBadDateAsOther:
-      properties:
-        badDate: {"type": "string", "format": "date", "default": 3}
-    WithBadDateTimeAsString:
-      properties:
-        badDate: {"type": "string", "format": "date-time", "default": "xxx"}
-    WithBadDateTimeAsOther:
-      properties:
-        badDate: {"type": "string", "format": "date-time", "default": 3}
-    WithBadUuidAsString:
-      properties:
-        badUuid: {"type": "string", "format": "uuid", "default": "xxx"}
-    WithBadUuidAsOther:
-      properties:
-        badUuid: {"type": "string", "format": "uuid", "default": 3}
-    WithBadEnum:
+    MyEnum:
+      type: string
+      enum: ["a", "A"]
+    MyModel:
       properties:
-        badEnum: {"type": "string", "enum": ["a", "b"], "default": "x"}
-"""
-        )
-    # Note, the null/None type, and binary strings (files), are not covered here due to a known bug:
-    # https://github.com/openapi-generators/openapi-python-client/issues/1162
-
-    @pytest.mark.parametrize(
-        ("model_name", "message"),
-        [
-            ("WithBadBoolean", "Invalid boolean value"),
-            ("WithBadIntAsString", "Invalid int value"),
-            ("WithBadIntAsOther", "Invalid int value"),
-            ("WithBadFloatAsString", "Invalid float value"),
-            ("WithBadFloatAsOther", "Cannot convert True to a float"),
-            ("WithBadDateAsString", "Invalid date"),
-            ("WithBadDateAsOther", "Cannot convert 3 to a date"),
-            ("WithBadDateTimeAsString", "Invalid datetime"),
-            ("WithBadDateTimeAsOther", "Cannot convert 3 to a datetime"),
-            ("WithBadUuidAsString", "Invalid UUID value"),
-            ("WithBadUuidAsOther", "Invalid UUID value"),
-            ("WithBadEnum", "Value x is not valid for enum"),
-        ]
-    )
-    def test_bad_default_warning(self, model_name, message, warnings):
-        assert_bad_schema_warning(warnings, model_name, message)
+        enumProp:
+          allOf:
+            - $ref: "#/components/schemas/MyEnum"
+          default: A
+""",
+    config="literal_enums: true",
+)
+@with_generated_code_imports(".models.MyModel")
+class TestLiteralEnumDefaults:
+    def test_default_value(self, MyModel):
+        assert MyModel().enum_prop == "A"