didx-xyz / aries-cloudapi-python

Apache License 2.0
12 stars 8 forks source link

add unit tests for endpoints in fastapi #124

Closed TimoGlastra closed 1 year ago

TimoGlastra commented 2 years ago

Currently we have unit tests for lower level methods, and we also make real api calls to test the public HTTP api of the cloudapi. I think it would be beneficial to also have unit tests for the CloudAPI endpoint methods.

This would make it possible to mock certain parts of the flow, making it easier to do setup for testing. Testing the issuer API currently needs interaction with the ledger, aca-py agent and the trust registry, which can be quite cumbersome to set up for every possible flow you want to test.

E.g. ACA-Py mocks that a request is made and calls the method that adds a route to the application directly: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/wallet/tests/test_routes.py

Maybe we can use mock requests (as described here) or actually just call the method with fake input data (e.g. get_credential("cred_id", acapy_controller_mock))

Don't want to do a big refactor and change everything without some input. What do you think? (mainly @morrieinmaas)

morrieinmaas commented 2 years ago

@TimoGlastra I agree with you here. Not saying this as an excuse but ,atter of factly, the way it is now has quite frankly been a way to cut corners a bit due to "lack of devs". Where possible there is unit and e2e being a mix of each other and where only unit tests possible there is only unit tests. That said, again, I agree ideally that should be as you describe.

TimoGlastra commented 2 years ago

Yeah I think integration tests are a good way to test the required flows, especially when low on resources. I'm not trying to bash the current state of things, just a thought on how we could improve testing over time :)

I've made a start at issuer unit tests in #125. Wasn't that much work once I, for the 1000th time, learned how to mock

morrieinmaas commented 2 years ago

Nice one for starting this in #125. Also went the mock way in trust-registry initial PR.

Maybe we do it this way from now on forward and then can refactor what's already there when there is some spare time/resources?