Proposal: run integration tests on PR
I would like to add a step to the PR workflow that runs some sort of integration tests so that we can identify any regressions before merging PRs. It would be nice if the tests where implemented right here in this repository. However, I think the integration tests in https://github.com/WebOfTrust/signify-ts are the most complete ones currently.
I will outline my simple proposal below. Any thoughts or alternative approaches?
Proposal - run signify-ts integration tests
I propose that we add a docker-compose environment similar to the one in signify-ts, it really only needs two small adjustments to run tests from here:
- The keria image would be built on the spot from the source code.
- We can build a "integration test image" also from the source code by cloning the signify-ts repo, I already have an example here: https://github.com/lenkan/signify-integration/blob/main/Dockerfile
Considerations
In case of planned breaking changes, such as changing a endpoint path or adding new required parameters, it would be obvious that the clients need updating ASAP in order to keep the builds healthy.
Another challenge is how to is the potential for false negatives. For example if a bad update to signify-ts causes tests to fail on PRs in keria. Could be solved by pinning the commit of the integration test suite, and only manually upgrade it when needed.
+1
Adding for consideration to the top 10 https://github.com/WebOfTrust/keri/discussions/78
Including the excellent test scenario from @lenkan here https://github.com/lenkan/keria-upgrade-test for consideration. It would be helpful if we can maintain/show a migration from previous versions to the latest version
I did something towards this here when creating a reproduction: https://github.com/lenkan/keria/tree/reproduction-contact-attributes-disappears
I think it can be extended to make it dead easy to run against local versions, debuggable versions as well as pushed docker images. You can also install multiple versions of signify.
I guess all of the time these have been e2e tests not integration. At least for me, integration is usually in between where you are still mocking some things or DB layers at a varying degree but higher level than unit.
My only worry about having Signify-TS e2e tests running in KERIA is this kind of circular dependency you kind of already mentioned - if you make a new feature in KERIA, you can't test it this way until it's implemented in Signify. And then the tests might never get added.
Ideally, though more work, we would have real integration tests in KERIA that operate at a higher level than unit but skip the Signify KERIA dance of HTTP requests and signed headers. If you have good coverage there you could be pretty sure you aren't breaking Signify.
And actually some of the KERIA tests are already like this, and some are more lower level. But there isn't a strong higher level set of tests in KERIA right now.
My only worry about having Signify-TS e2e tests running in KERIA is this kind of circular dependency you kind of already mentioned - if you make a new feature in KERIA, you can't test it this way until it's implemented in Signify. And then the tests might never get added.
Yes, I agree that is not ideal. The problem I want to solve is to ensure updates to keria are compatible with signify. I want to avoid the situation we had for several months this year when the integration was broken.
Anyway, for now, that example was just to reproduce an issue. I also think we need to iterate a few times before considering it for anything real.
And actually some of the KERIA tests are already like this, and some are more lower level. But there isn't a strong higher level set of tests in KERIA right now.
Perhaps improving those tests is sufficient then. It will not solve the problem with incompatible client libraries though.
@lenkan Would an approach to having builds and automated integration tests for signify-ts run against a specific KERIA build (git commit hash or version), versus always using the latest be a better way to address this problem?
For testing a breaking change that needs to be coordinated, this could first be tested on a branch of signify-ts.
The problem I want to solve is to ensure updates to keria are compatible with signify. I want to avoid the situation we had for several months this year when the integration was broken.
If there's a planned breaking change, then the tests won't pass though. So we'd have to handle that, and I do agree that they were broken for far too long. Part of this was KERIA was actually broken from keripy too.
Perhaps improving those tests is sufficient then. It will not solve the problem with incompatible client libraries though.
Yeah agreed. I mean it should catch most things aside from API changes, and API changes should just be a Signify upgrade.
But in general it'd be good if there weren't too many breaking changes even if planned. I'm already worried about having a Signify mobile app on an app store and installed versions being out of sync with KERIA providers people are using.
@edeykholt Yeah that was a problem but has already been fixed - we use dev releases now. That keeps Signify stable but I mean if KERIA changed dramatically and we needed to upgrade the version for some other reason, then Signify would be broken again.
There's no perfect solution for this except for a monorepo with the client repos and coordinated contributors to both KERIA and Signify, and that only works well if there's only 1 client library imo. But in general monorepos work well for that because you can't merge the breaking changes without at least fixing the other library.