Client Spec Test Rewrite

[sighphyre]

What is this?

This is a full rewrite of the current client specification tests to use the client specifications project, rather than a custom implementation of each test. This should load each specification test individually from file and execute them independently, while retaining the test name included in the spec file, so that when failures are picked up, we can know what's broken and why. This leverages the bootstrapping capability of this SDK to ensure data contracts are correct, but ignores the API level. Our plan to ensure API level correctness through an upcoming project across all of our officially supported SDKs.

Why was this done?

There's a few reasons why this is a useful thing to do:

• Adding new test cases when Unleash adds new features is time consuming and error prone, we're planning on adding further tests in the near future to extend test case coverage to ensure that each of the official SDKs correctly support the edge cases and new features, I don't believe that anyone has appetite to replicate this work in the Python client
• On the Unleash side we're building an integration testing tool to validate all our SDKs against server API responses, while the approach in this project is not blocking to that, it has picked up a number of edge case failures
• Previously, this was tough to do, but with the addition of bootstrapping and the ability to set the client to read only mode this becomes much easier to tackle without mocking out the API responses

Why is this a draft?

We've picked up a few test failures here, the plan is to dig a little deeper into those failures in the near future and either patch our specs if they're false negatives or fix the underlying issues

[sighphyre]

I feel like this is a good idea. Though, how does a test failure look with this new setup, is it still easy to see which tests are failing?

This is a good question! This was important to to me because a few of our SDKs are badly behaved around reporting failures. Taken right from my console:

FAILED tests/specification_tests/test_client_specs.py::test_spec[09-strategy-constraints-Feature.constraints.multi should be enabled in prod for user 2] - AssertionError: assert False == True
FAILED tests/specification_tests/test_client_specs.py::test_spec[09-strategy-constraints-Feature.constraints.custom should be enabled in prod for norway] - AssertionError: assert False == True
FAILED tests/specification_tests/test_client_specs.py::test_spec[09-strategy-constraints-Feature.constraints.dual should be enabled in prod] - AssertionError: assert False == True
FAILED tests/specification_tests/test_client_specs.py::test_spec[09-strategy-constraints-Enabled in prod for userId=123] - AssertionError: assert False == True
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_100 should be enabled without field defined for 100%] - AssertionError: assert ...
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_50 should be enabled for customField=388] - AssertionError: assert False == True
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_50 should be enabled for customField=390] - AssertionError: assert False == True
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_50 should be enabled for customField=391] - AssertionError: assert {'enabled': ...
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_50 should be enabled for customField=392] - AssertionError: assert {'enabled': ...
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_50 should be enabled for customField=393] - AssertionError: assert {'enabled': ...
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_50 should be enabled for customField=394] - AssertionError: assert {'enabled': ...
FAILED tests/specification_tests/test_client_specs.py::test_spec[12-custom-stickiness-Feature.flexible.rollout.custom.stickiness_50 should be enabled for customField=395] - AssertionError: assert {'enabled': ...
FAILED tests/specification_tests/test_client_specs.py::test_spec[13-constraint-operators-F5.numEq] - AssertionError: assert False == True
FAILED tests/specification_tests/test_client_specs.py::test_spec[13-constraint-operators-F5.numEq works for floats] - AssertionError: assert False == True

So fairly similar to the Node SDK. Pytest will also report the exception but it's not all that useful:

________________________________________________________________________ test_spec[13-constraint-operators-F7.dateAfter should be enabled] ________________________________________________________________________

spec = (<UnleashClient.UnleashClient object at 0x7fb3281d6020>, {'context': {'appName': 'pytest', 'currentTime': '2022-01-30T...fault'}, 'description': 'F7.dateAfter should be enabled', 'expectedResult': True, 'toggleName': 'F7.dateAfter'}, False)

    @pytest.mark.parametrize("spec", TEST_DATA, ids=TEST_NAMES)
    def test_spec(spec):
        unleash_client, test_data, is_variant_test = spec
        if not is_variant_test:
            toggle_name = test_data["toggleName"]
            expected = test_data["expectedResult"]
            context = test_data.get("context")
>           assert unleash_client.is_enabled(toggle_name, context) == expected
E           AssertionError: assert False == True
E            +  where False = <bound method UnleashClient.is_enabled of <UnleashClient.UnleashClient object at 0x7fb3281d6020>>('F7.dateAfter', {'appName': 'pytest', 'currentTime': '2022-01-30T13:00:00.000Z', 'environment': 'default'})
E            +    where <bound method UnleashClient.is_enabled of <UnleashClient.UnleashClient object at 0x7fb3281d6020>> = <UnleashClient.UnleashClient object at 0x7fb3281d6020>.is_enabled

[chriswk]

That seems like a good ux though. Then it's on the spec writers to keep the cases clear.

[coveralls]

Coverage increased (+0.05%) to 96.184% when pulling 87eb23032370953f6f469d6a99bcd49f97c3aeda on rewrite-client-specifications into 686307613afb1f9129d6510e1d750fb21ee8b2a8 on main.