adjusted summary report (#553)

Author: ammokhov

.pre-commit-config.yaml

@@ -11,7 +11,7 @@ repos:
   - id: isort
     # language_version: python3.6
 - repo: https://github.com/ambv/black
-  rev: stable
+  rev: 20.8b1
   hooks:
   - id: black
 - repo: https://github.com/pre-commit/pre-commit-hooks

README.md

@@ -40,6 +40,7 @@ To run the contract tests, use the `test` command.
 ```bash
 cfn test
 cfn test -- -k contract_delete_update #to run a single test
+cfn test --tb=long # exhaustive, informative traceback formatting
 cfn test --enforce-timeout 60 #set the RL handler timeout to 60 seconds and CUD handler timeout to 120 seconds.
 cfn test --enforce-timeout 60 -- -k contract_delete_update # combine args
 ```

src/rpdk/core/contract/resource_client.py

@@ -204,7 +204,8 @@ def assert_write_only_property_does_not_exist(self, resource_model):
             assert not any(
                 self.key_error_safe_traverse(resource_model, write_only_property)
                 for write_only_property in self.write_only_paths
-            )
+            ), "The model MUST NOT return properties defined as \
+                writeOnlyProperties in the resource schema"
 
     @property
     def strategy(self):
@@ -378,22 +379,37 @@ def assert_time(self, start_time, end_time, action):
 
     @staticmethod
     def assert_primary_identifier(primary_identifier_paths, resource_model):
-        assert all(
-            traverse(resource_model, fragment_list(primary_identifier, "properties"))[0]
-            for primary_identifier in primary_identifier_paths
-        )
+        try:
+            assert all(
+                traverse(
+                    resource_model, fragment_list(primary_identifier, "properties")
+                )[0]
+                for primary_identifier in primary_identifier_paths
+            ), "Every returned model MUST include the primaryIdentifier"
+        except KeyError as e:
+            raise AssertionError(
+                "Every returned model MUST include the primaryIdentifier"
+            ) from e
 
     @staticmethod
     def is_primary_identifier_equal(
         primary_identifier_path, created_model, updated_model
     ):
-        return all(
-            traverse(created_model, fragment_list(primary_identifier, "properties"))[0]
-            == traverse(updated_model, fragment_list(primary_identifier, "properties"))[
-                0
-            ]
-            for primary_identifier in primary_identifier_path
-        )
+        try:
+            return all(
+                traverse(
+                    created_model, fragment_list(primary_identifier, "properties")
+                )[0]
+                == traverse(
+                    updated_model, fragment_list(primary_identifier, "properties")
+                )[0]
+                for primary_identifier in primary_identifier_path
+            )
+        except KeyError as e:
+            raise AssertionError(
+                "The primaryIdentifier returned in every progress event must\
+                     match the primaryIdentifier passed into the request"
+            ) from e
 
     def _make_payload(self, action, current_model, previous_model=None, **kwargs):
         return self.make_request(

src/rpdk/core/contract/suite/contract_asserts.py

@@ -1,7 +1,6 @@
 import logging
 from functools import wraps
 from inspect import Parameter, signature
-from unittest.test.testmock.support import is_instance
 
 import pytest
 
@@ -85,7 +84,11 @@ def response_does_not_contain_write_only_properties(resource_client, response):
 def response_contains_resource_model_equal_current_model(
     response, current_resource_model
 ):
-    assert response["resourceModel"] == current_resource_model
+    assert (
+        response["resourceModel"] == current_resource_model
+    ), "All properties specified in the request MUST be present in the model \
+        returned, and they MUST match exactly, with the exception of properties\
+             defined as writeOnlyProperties in the resource schema"
 
 
 @decorate()
@@ -95,7 +98,9 @@ def response_contains_resource_model_equal_updated_model(
     assert response["resourceModel"] == {
         **current_resource_model,
         **update_resource_model,
-    }
+    }, "All properties specified in the update request MUST be present in the \
+        model returned, and they MUST match exactly, with the exception of \
+            properties defined as writeOnlyProperties in the resource schema"
 
 
 @decorate()
@@ -109,11 +114,12 @@ def response_contains_primary_identifier(resource_client, response):
 def response_contains_unchanged_primary_identifier(
     resource_client, response, current_resource_model
 ):
-    resource_client.is_primary_identifier_equal(
+    assert resource_client.is_primary_identifier_equal(
         resource_client.primary_identifier_paths,
         current_resource_model,
         response["resourceModel"],
-    )
+    ), "PrimaryIdentifier returned in every progress event must match \
+        the primaryIdentifier passed into the request"
 
 
 @decorate(after=False)
@@ -128,7 +134,7 @@ def decorator_wrapper(func: object):
         def wrapper(*args, **kwargs):
             response_error = func(*args, **kwargs)
             if response_error is not None:
-                if is_instance(error_code, HandlerErrorCode):
+                if isinstance(error_code, HandlerErrorCode):
                     error_code_tuple = (error_code,)
                 assert response_error in error_code_tuple, msg
             return response_error

src/rpdk/core/contract/suite/handler_commons.py

@@ -29,7 +29,8 @@ def test_create_success(resource_client, current_resource_model):
 
 @failed_event(
     error_code=HandlerErrorCode.AlreadyExists,
-    msg="creating the same resource should not be possible",
+    msg="A create handler MUST NOT create multiple resources given\
+         the same idempotency token",
 )
 def test_create_failure_if_repeat_writeable_id(resource_client, current_resource_model):
     LOG.debug(
@@ -59,8 +60,15 @@ def test_read_success(resource_client, current_resource_model):
     return response
 
 
-@failed_event(error_code=HandlerErrorCode.NotFound)
-def test_read_failure_not_found(resource_client, current_resource_model):
+@failed_event(
+    error_code=HandlerErrorCode.NotFound,
+    msg="A read handler MUST return FAILED with a NotFound error code\
+         if the resource does not exist",
+)
+def test_read_failure_not_found(
+    resource_client,
+    current_resource_model,
+):
     primay_identifier_only_model = create_model_with_properties_in_path(
         current_resource_model,
         resource_client.primary_identifier_paths,
@@ -115,7 +123,11 @@ def test_update_success(resource_client, update_resource_model, current_resource
     return response
 
 
-@failed_event(error_code=HandlerErrorCode.NotFound)
+@failed_event(
+    error_code=HandlerErrorCode.NotFound,
+    msg="An update handler MUST return FAILED with a NotFound error code\
+         if the resource did not exist prior to the update request",
+)
 def test_update_failure_not_found(resource_client, current_resource_model):
     update_model = resource_client.generate_update_example(current_resource_model)
     _status, _response, error_code = resource_client.call_and_assert(
@@ -135,7 +147,11 @@ def test_delete_success(resource_client, current_resource_model):
     return response
 
 
-@failed_event(error_code=HandlerErrorCode.NotFound)
+@failed_event(
+    error_code=HandlerErrorCode.NotFound,
+    msg="A delete hander MUST return FAILED with a NotFound error code\
+         if the resource did not exist prior to the delete request",
+)
 def test_delete_failure_not_found(resource_client, current_resource_model):
     primay_identifier_only_model = create_model_with_properties_in_path(
         current_resource_model,

src/rpdk/core/contract/suite/handler_delete.py

@@ -42,15 +42,20 @@ def deleted_resource(resource_client):
         _status, response, _error = resource_client.call_and_assert(
             Action.DELETE, OperationStatus.SUCCESS, primay_identifier_only_model
         )
-        assert "resourceModel" not in response
+        assert (
+            "resourceModel" not in response
+        ), "The deletion handler's response object MUST NOT contain a model"
         yield model, request
     finally:
         status, response = resource_client.call(Action.DELETE, pruned_model)
 
         # a failed status is allowed if the error code is NotFound
         if status == OperationStatus.FAILED:
             error_code = resource_client.assert_failed(status, response)
-            assert error_code == HandlerErrorCode.NotFound
+            assert (
+                error_code == HandlerErrorCode.NotFound
+            ), "A delete hander MUST return FAILED with a NotFound error code\
+                 if the resource did not exist prior to the delete request"
         else:
             resource_client.assert_success(status, response)
 
@@ -70,7 +75,10 @@ def contract_delete_list(resource_client, deleted_resource):
     #       remove the model from the list, however.
 
     deleted_model, _request = deleted_resource
-    assert not test_model_in_list(resource_client, deleted_model)
+    assert not test_model_in_list(
+        resource_client, deleted_model
+    ), "A list operation MUST NOT return the primaryIdentifier \
+        of any deleted resource instance"
 
 
 @pytest.mark.delete
@@ -99,7 +107,11 @@ def contract_delete_create(resource_client, deleted_resource):
             response["resourceModel"], resource_client.read_only_paths
         )
 
-        assert deleted_model == response["resourceModel"]
+        assert (
+            deleted_model == response["resourceModel"]
+        ), "Once a delete operation successfully completes, a subsequent\
+             create request with the same primaryIdentifier or additionalIdentifiers\
+                  MUST NOT return FAILED with an AlreadyExists error code"
         resource_client.call_and_assert(
             Action.DELETE, OperationStatus.SUCCESS, created_response["resourceModel"]
         )

src/rpdk/core/contract/suite/handler_update.py

@@ -51,7 +51,8 @@ def contract_update_read_success(updated_resource, resource_client):
     _create_request, _created_model, _update_request, updated_model = updated_resource
     assert resource_client.is_primary_identifier_equal(
         resource_client.primary_identifier_paths, _created_model, updated_model
-    )
+    ), "The primaryIdentifier returned must match\
+         the primaryIdentifier passed into the request"
     test_read_success(resource_client, updated_model)
 
 
@@ -63,5 +64,8 @@ def contract_update_list_success(updated_resource, resource_client):
     _create_request, _created_model, _update_request, updated_model = updated_resource
     assert resource_client.is_primary_identifier_equal(
         resource_client.primary_identifier_paths, _created_model, updated_model
-    )
-    assert test_model_in_list(resource_client, updated_model)
+    ), "The primaryIdentifier returned must match\
+         the primaryIdentifier passed into the request"
+    assert test_model_in_list(
+        resource_client, updated_model
+    ), "A list handler MUST always return an updated model"

src/rpdk/core/contract/suite/handler_update_invalid.py

@@ -11,7 +11,13 @@
 
 
 @pytest.mark.update
-@failed_event(error_code=(HandlerErrorCode.NotUpdatable, HandlerErrorCode.NotFound))
+@failed_event(
+    error_code=(HandlerErrorCode.NotUpdatable, HandlerErrorCode.NotFound),
+    msg="An update hander MUST return FAILED with a NotUpdatable error code \
+        if the difference between the previous state and the properties included \
+            in the request contains a property defined in the createOnlyProperties \
+                in the resource schema",
+)
 def contract_update_create_only_property(resource_client):
 
     if resource_client.create_only_paths:
@@ -27,7 +33,10 @@ def contract_update_create_only_property(resource_client):
             _status, response, _error_code = resource_client.call_and_assert(
                 Action.UPDATE, OperationStatus.FAILED, update_request, created_model
             )
-            assert response["message"]
+            assert response[
+                "message"
+            ], "The progress event MUST return an error message\
+                 when the status is failed"
         finally:
             primay_identifier_only_model = create_model_with_properties_in_path(
                 created_model,
@@ -43,13 +52,17 @@ def contract_update_create_only_property(resource_client):
 @pytest.mark.update
 @failed_event(
     error_code=HandlerErrorCode.NotFound,
-    msg="cannot update a resource which does not exist",
+    msg="An update handler MUST return FAILED with a NotFound error code\
+         if the resource did not exist prior to the update request",
 )
 def contract_update_non_existent_resource(resource_client):
     create_request = resource_client.generate_invalid_create_example()
     update_request = resource_client.generate_update_example(create_request)
     _status, response, _error = resource_client.call_and_assert(
         Action.UPDATE, OperationStatus.FAILED, update_request, create_request
     )
-    assert response["message"]
+    assert response[
+        "message"
+    ], "The progress event MUST return an error message\
+         when the status is failed"
     return _error

src/rpdk/core/data/pytest-contract.ini

@@ -1,8 +1,11 @@
 [pytest]
+log_cli = True
+log_cli_level = INFO
 addopts =
     --pyargs "rpdk.core.contract.suite"
     --verbose
     --show-capture no
+    --tb=short
 python_files = handler_*.py
 python_functions = contract_*
 markers =

tests/contract/test_resource_client.py

@@ -804,7 +804,7 @@ def test_assert_primary_identifier_success(resource_client):
 
 
 def test_assert_primary_identifier_fail(resource_client):
-    with pytest.raises(KeyError):
+    with pytest.raises(AssertionError):
         resource_client._update_schema(SCHEMA)
         resource_client.assert_primary_identifier(
             resource_client.primary_identifier_paths, {"a": 1, "b": 2}
@@ -829,6 +829,16 @@ def test_is_primary_identifier_equal_fail(resource_client):
     )
 
 
+def test_is_primary_identifier_equal_fail_key(resource_client):
+    with pytest.raises(AssertionError):
+        resource_client._update_schema(SCHEMA)
+        resource_client.is_primary_identifier_equal(
+            resource_client.primary_identifier_paths,
+            {"a": 1, "b": 2},
+            {"a": 1, "b": 2},
+        )
+
+
 def test_assert_write_only_property_does_not_exist(resource_client):
     schema = {
         "a": {"type": "number", "const": 1},