search

guardian / path-manager

Service that manages paths for a domain. Ensures they are unique etc.
2 stars 0 forks source link

add PathStore integration tests (using DynamoDB docker image) #29

Closed twrichards closed 4 years ago

twrichards commented 4 years ago

https://trello.com/c/dGQVgKRl/293-introduce-an-integration-test-harness-for-path-manager-endpoints-prior-to-refactoring

Problem

In order to implement the new 'aliasing' feature in PathManager, we must use 'transactions' which weren't available in DynamoDB when Path Manager was first implemented. In https://trello.com/c/GH4sBqFx/290-transactions-in-path-manager we'll be adding support for transactions and refactoring any existing 'transaction-like' logic. Right now there are no integration tests at all, so we don't have heaps of confidence for refactoring.

Solution

Add integration tests around all 'transaction-like' logic to ensure we have confidence going into refactoring.

Definition of Success

Passing 🆕 integration tests in CI.

What does this change?

This PR introduces https://github.com/whisklabs/docker-it-scala into this repo and implemented reusable trait (DockerDynamoTestBase) so we can write tests against a local DynamoDB instance.

Using the newly added DockerDynamoTestBase added tests for...

In order for these tests to work in TeamCity the DynamoDB client instantiation had to be updated because it distinguished between connecting to local DynamoDB or a AWS hosted DynamoDB based on whether it was running on an EC2 instance, which since TeamCity agents are EC2 instances the new tests were exploding in CI, so now it checks whether the Stage tag is not DEVINFRA which means the new tests now run in TeamCity... image

How to test

sbt test

How can we measure success?

With these tests in place we have greater confidence going into refactoring these functions to make use of DynamoDB 'transactions'.

Have we considered potential risks?

These changes just affect local and CI, rather than the running application, so minimal/no increased risk.

Images

N/A

jonathonherbert commented 4 years ago

This looks great. It's nice to see how slim the scaffolding for the Docker work is.

I'll wait for others to comment on the domain-specific aspects, and for your todo re: testing in CODE, before approving.

twrichards commented 4 years ago
twrichards commented 4 years ago

Turns out there is an issue with something in this PR such that boxes cycle in CODE, after SSHing onto one of the boxes this is the errors in /var/log/syslog... image ...it would be strange if the addition of two test dependencies and the removal of two others would affect this here, but not locally

sihil commented 4 years ago

Ugg. That's not a helpful error. Initialisation is slightly different in the local dev and test modes, particularly in older versions of Play. I'd strongly suggest switching to compile time DI and bumping to at least Play 2.5 which I think kills off the global object. I'd then expect this kind of issue to blow up locally.

I guess that biting the bullet and bumping Play etc is probably something we'd want to do anyway.

twrichards commented 4 years ago

Ugg. That's not a helpful error. Initialisation is slightly different in the local dev and test modes, particularly in older versions of Play. I'd strongly suggest switching to compile time DI and bumping to at least Play 2.5 which I think kills off the global object. I'd then expect this kind of issue to blow up locally.

I guess that biting the bullet and bumping Play etc is probably something we'd want to do anyway.

I decided to go for a mega upgrade of everything to (more/less) latest - https://github.com/guardian/path-manager/pull/32 - and whilst it needs a bit more effort its working in a basic form and having rebased this branch onto that one, this is now deploying and working in CODE 🎉