EngineerBetter / concourse-up

Deprecated - used Control Tower instead
https://github.com/EngineerBetter/control-tower
Apache License 2.0
203 stars 28 forks source link

Refactoring tests for maintainability #14

Closed elgohr closed 5 years ago

elgohr commented 6 years ago

Looking forward to add OpenStack support.

In this way I refactored tests first, to improve maintainability. Summary:

peterellisjones commented 6 years ago

Hi @elgohr,

Thanks for you PR and apologies about the late reply on this. We're still discussing the merits of code generation and will hopefully have an answer soon.

Mocking interaction with AWS in aws_hosted_zone_test. This test seemed to be some kind of integration test. Nevertheless nobody else got your credentials for executing the tests. I would expect dedicated integration tests to happen in CI (but didn't find any). Any plans?

For the aws_hosted_zone_test yes you're right that you'll need to use your own AWS credentials (we can't share them on a public repo for security reasons). We don't have specific integration tests but there is a suite of system tests we run which you can find here

cheers,

Pete

elgohr commented 6 years ago

We don't have specific integration tests but there is a suite of system tests we run which you can find here

To me https://github.com/EngineerBetter/concourse-up/blob/master/ci/tasks/system-test-alt-region.sh looks like an integration test for AWS. Can't we remove aws_hosted_zone_test.go in this way? The system test is doing the same and more... Or is it just to verify that errors from AWS are returned from the method?

crsimmons commented 5 years ago

This PR is quite old and now has merge conflicts. We have decided not to go with code generation at this time so I am closing this PR.