Closed skalish closed 4 years ago
Great PR!
Note: In my mind, coverage is not the end-goal but rather a means towards the end-goal of having tests that give you confidence in your codebase. So no one should mistake this PR as acceptance of coverage % as something we wish to optimize. Rather, we should write tests that exercise features as close to real usage in the wild as possible and write tests only if they increase our confidence in our code. Writing a test that increases coverage but tests something that is trivially correct just adds to maintenance without increasing confidence.
^Mostly wrote this for onlookers as I think @skalish and I are already on the same page 😄
↪️ Pull Request
Quick PR to improve test coverage of
tamr_client
.For the most part, all mock server requests are now present. Testing is not included for the synchronous workflow functions (core asynchronous versions do have testing), since these are only meaningful when tested against a real Tamr instance. For instance, this PR adds mock server request/response tests related to #437, but does not satisfy the need for end-to-end testing.
Additionally, the
description
property ofSubAttributes
has been removed to align with the restrictions on the related objects in Tamr.✔️ PR Todo