restore some missing test coverage

Author: eli-bl

.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.

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
 

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)

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:

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,
         )
 
 
@@ -86,72 +89,3 @@ def test_defaults_in_initializer(self, MyModel):
 class TestEnumDefaults:
     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(
-"""
-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:
-      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)

end_to_end_tests/generated_code_live_tests/test_enum_and_const.py renamed to end_to_end_tests/generated_code_live_tests/test_enums_and_consts.py

@@ -1,11 +1,6 @@
-
-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,
-    inline_spec_should_fail,
     with_generated_code_import,
     with_generated_client_fixture,
     with_generated_code_imports,
@@ -133,52 +128,89 @@ def test_invalid_int(self, MyModel):
             MyModel.from_dict({"mustBeThirty": 29})
 
 
-class TestEnumAndConstInvalidSchemas:
-    @pytest.fixture(scope="class")
-    def warnings(self):
-        return inline_spec_should_cause_warnings(
+# The following tests of literal enums use basically the same specs as the tests above, but
+# the "literal_enums" option is enabled in the test configuration.
+
+@with_generated_client_fixture(
 """
 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")
+    MyEnum:
+      type: string
+      enum: ["a", "A"]
+    MyIntEnum:
+      type: integer
+      enum: [2, 3]
+    MyEnumIncludingNull:
+      type: ["string", "null"]
+      enum: ["a", "b", null]
+    MyNullOnlyEnum:
+      enum: [null]
+    MyModel:
+      properties:
+        enumProp: {"$ref": "#/components/schemas/MyEnum"}
+        intEnumProp: {"$ref": "#/components/schemas/MyIntEnum"}
+        nullableEnumProp:
+          oneOf:
+            - {"$ref": "#/components/schemas/MyEnum"}
+            - type: "null"
+        enumIncludingNullProp: {"$ref": "#/components/schemas/MyEnumIncludingNull"}
+        nullOnlyEnumProp: {"$ref": "#/components/schemas/MyNullOnlyEnum"}
+        inlineEnumProp:
+          type: string
+          enum: ["a", "b"]
+""",
+    config="""
+literal_enums: true
+""",
+)
+@with_generated_code_import(".models.MyModel")
+class TestLiteralEnums:
+    def test_enum_prop(self, MyModel):
+        assert_model_decode_encode(MyModel, {"enumProp": "a"}, MyModel(enum_prop="a"))
+        assert_model_decode_encode(MyModel, {"enumProp": "A"}, MyModel(enum_prop="A"))
+        assert_model_decode_encode(MyModel, {"intEnumProp": 2}, MyModel(int_enum_prop=2))
+        assert_model_decode_encode(MyModel, {"inlineEnumProp": "a"}, MyModel(inline_enum_prop="a"))
+
+    def test_enum_prop_type(self, MyModel):
+        assert MyModel.from_dict({"enumProp": "a"}).enum_prop.__class__ is str
+        assert MyModel.from_dict({"intEnumProp": 2}).int_enum_prop.__class__ is int
+
+    def test_nullable_enum_prop(self, MyModel):
+        assert_model_decode_encode(MyModel, {"nullableEnumProp": "B"}, MyModel(nullable_enum_prop="B"))
+        assert_model_decode_encode(MyModel, {"nullableEnumProp": None}, MyModel(nullable_enum_prop=None))
+        assert_model_decode_encode(MyModel, {"enumIncludingNullProp": "a"}, MyModel(enum_including_null_prop="a"))
+        assert_model_decode_encode(MyModel, {"enumIncludingNullProp": None}, MyModel(enum_including_null_prop=None))
+        assert_model_decode_encode(MyModel, {"nullOnlyEnumProp": None}, MyModel(null_only_enum_prop=None))
 
-    def test_enum_unsupported_type(self, warnings):
-        assert_bad_schema_warning(warnings, "WithUnsupportedType", "Unsupported enum type")
+    def test_invalid_values(self, MyModel):
+        with pytest.raises(TypeError):
+            MyModel.from_dict({"enumProp": "c"})
+        with pytest.raises(TypeError):
+            MyModel.from_dict({"enumProp": 2})
+        with pytest.raises(TypeError):
+            MyModel.from_dict({"intEnumProp": 0})
+        with pytest.raises(TypeError):
+            MyModel.from_dict({"intEnumProp": "a"})
 
-    def test_const_default_not_matching(self, warnings):
-        assert_bad_schema_warning(warnings, "DefaultNotMatchingConst", "Invalid value for const")
 
-    def test_enum_duplicate_values(self):
-        # This one currently causes a full generator failure rather than a warning
-        result = inline_spec_should_fail(
+@with_generated_client_fixture(
 """
 components:
   schemas:
-    WithDuplicateValues:
-      enum: ["x", "x"]
-"""
-        )
-        assert "Duplicate key X in enum" in str(result.exception)
+    MyEnum:
+      type: string
+      enum: ["a", "A"]
+    MyModel:
+      properties:
+        enumProp:
+          allOf:
+            - $ref: "#/components/schemas/MyEnum"
+          default: A
+""",
+    config="literal_enums: true",
+)
+@with_generated_code_import(".models.MyModel")
+class TestLiteralEnumDefaults:
+    def test_default_value(self, MyModel):
+        assert MyModel().enum_prop == "A"