How to perform CI testing with integration tests?

[ikehz]

Per #18.

I've written an integration test framework and suite of tests that work with a live integration setup, and we need to figure out how to get those tests to not fail on Travis CI. We need these tests run regularly to make sure that the library actually works against the current API. Options I can think of:

1. We could upload the cassettes as part of the codebase. This is contrary to Introduction of VCR has broken live tests fog#2112, and the problems are numerous. The two big ones are:
   • We'll have to commit VCR cassettes into the codebase. That's a huge pain, and creates messy commits.
   • We or someone else might inadvertently commit sensitive information embedded in the cassettes, (e.g. authentication information).
2. We can link tests to mocks. This is also not a great solution, since it means:
   • We have to keep the mocks up-to-date
   • We might get the CI test passing when it should fail against the live API. This is how we got to where we are now, where we don't know how much of the codebase still works and how much is built against an old API spec.
3. We can run tests against a live project. This also has numerous issues:
   • If a test fails, it's (very) hard to clean up after, (i.e. it may have created resources that it didn't delete, so we might end up with VMs or other things lying around which will cause problems with future tests and also rack up costs for anyone else testing this stuff).
   • It means putting credentials for a project up on our Travis CI server. I don't know how secure that is.
   • They take a very long time to run, (at least 30 minutes,) and they aren't particularly consistent, (e.g. a bad connection can make a whole suite fail for unclear reasons).
4. We can not run integration tests on Travis CI, and run them in some other environment, where we can store credentials. The brain-dead solution would be to just have me, (or someone else,) run them locally at every update. This is neither transparent nor sustainable.

@plribeiro3000 Your thoughts would be helpful here. Does the Fog community, as far as you know, have working solutions to this problem?

@erjohnso Do you have suggestions, based on how other projects are doing this?

[plribeiro3000]

Hey @ihmccreery. What i did in fog-xenserver was to commit the VCR cassetes. As you mentioned it could lead to several issues as it is it and i agree we might have to come up with a better solution if possible.

Also i think that the option 1 is the one which best replicates production without losing sustainability.

But i would like to further discuss it. =)

[ikehz]

Has anyone explored, as far as you know, using secure variables on Travis to run live integration tests? Of course this will not run on untrusted builds, (i.e. not on pull requests,) so we still need to cover code with unit tests, but it provides some level of live integration test functionality, which is nice.

[ikehz]

@tokengeek I know you've wrestled with this before, (fog/fog#2112). If you have input, please chime in.

[plribeiro3000]

@ihmccreery Not that i´m aware of. The main issue is that we can´t run tests on the PR´s thus we don´t know if it is ok. =s

EDIT: Even with this secure feature, a malicious user will be able to access your provider and create unintended stuff there.

[ikehz]

@plribeiro3000 Can you say more about your previous edit, "... a malicious user will be able to access your provider and create unintended stuff there"?

[plribeiro3000]

Sure. An user will be able to create stuff on your account. They have fog and access to your account so a user can create a vm there.

[ikehz]

Hm. I'm not sure I understand. At least in theory, we should be able to encrypt our credentials using the secure variables on Travis, so that they are not available to anyone except those who have admin access to the Travis account, (i.e. committers to fog-google). Travis can decrypt them to run tests, but, as long as no malicious code is checked in and run, (which, albeit, is more complicated than it sounds,) no one can see the decrypted versions. Am I missing something?

[plribeiro3000]

Yeah. The user won´t be able to get your credentials but it will be able to create a vm, ssh into it and push some malicious code there. Thanks to fog, it won´t be necessary to know your credentials if he can create a script with its permissions.

Instead of code, the person can submit this code:

conn = # open connection
server = conn.servers.create(options)
server.ssh do
  # some malicious stuff
  # like host a porn website
end

Until you get to it, your account will be hosting bad stuff.

[ikehz]

Doesn't this prevent that, as long as we don't pull that code into the main repo?

Secure variables will not be added to untrusted builds (ie, builds for pull requests coming from another repository).

(From the docs).

[icco]

@ihmccreery yes, that's what @plribeiro3000 is saying. Since Google has a free tier (or if they just want to give us a whitelisted account with small quota), I think this is the best solution. Things that will happen that are probably reasonable risks:

• Any code written and merged to master will execute against production GCE. This is true already, since some people run fog-google from head without reading every patch.
• Members of the team that runs Travis CI could in theory access those keys and do something malicious with them (which is why I suggest keeping a low quota on the account).

[plribeiro3000]

@ihmccreery I think the point is: if you don't run the tests on unstrusted builds (builds for open Pull Requests) how are you going to check if the tests pass for the patch submitted?

If you have to go and run the tests by yourself then we are going to loose the advantage of having a CI. =s

The only two ways i can think of to solve this is to use an automated tool like VCR or have an account with with bare minimum access to test it. But even that account might not be enough if you want to test a feature that account does not have access. =s

Just like a friend of mine always say:

Programming is hard!

cc/ @matiasleidemer

[plribeiro3000]

I'm not sure how we should handle this kind of testing. I feel that sometimes we miss some tests accessing the api for real but i don't feel safe in any of the possible ways to do so.

Perhaps we could use VCR or any other solution of the type just for critical apis? And let simple ones just with simple mocks (webmock)?

[ikehz]

The correct way to do this, in my opinion, is to run unit & functional
tests on all builds, (including PRs,) and run full integration tests just
against trusted builds, (i.e. master, and occasionally trusted PRs,
maybe). There is a common problem in large-scale software engineering:
it's infeasible to run full integration tests against all builds, (even
trusted ones,) simply because they take too long. The usual solution is
that unit & functional tests are run against everything, and full
integration only runs against master.

The fact is, though, that the correct way is not always feasible—unit &
functional tests right now don't give us very helpful signal, and no one
seems to have the time to fix them, myself included. Given the low volume
of PRs against this repository, I think the best we'll do is run unit &
functional, (which are kind of throw-away at this point,) and run
integration against master & trusted builds.

Make sense?

On Tue, Aug 4, 2015 at 8:26 AM, Paulo Henrique Lopes Ribeiro <
notifications@github.com> wrote:

I'm not sure how we should handle this kind of testing. I feel that
sometimes we miss some tests accessing the api for real but i don't feel
safe in any of the possible ways to do so.

Perhaps we could use VCR or any other solution of the type just for
critical apis? And let simple ones just with simple mocks (webmock)?

—
Reply to this email directly or view it on GitHub
#19 (comment).

[plribeiro3000]

Yeah. i do agree. The only issue with your suggestion is how that would help my PR (#57) not break everything without a merge.
Usually the kinds of things that might break a feature are refactoring's and as such we should use PR's to help review it. But then you don't have the feedback of the tests that hit the api for real. =s

Lets try to make it simpler before we actually take some action. Why is VCR such a bad option that makes all the efforts and consequences to run integration tests only on master a better option?

[selmanj]

Can we reopen this discussion?

I thought I might take a crack at updating the google-api-client to 0.9, but I'm unable to test it easily. Specifically, all the tests run against the mock implementations. There's some integration code in the examples/ directory, but it's very annoying to run (I have to configure credentials, then run individually via 'bundle exec ruby ' after modifying the file itself to disable mocking). It would be nice to have VCR functionality for some sort of integration test that at least verifies that everything is working beyond the mocks. Is anyone opposed to having some sort of VCR setup?

[Temikus]

@selmanj The problem with VCR is that the tapes bitrot quite fast and don't catch errors or changes on the backend. I would rather see someone fix the integration tests to be honest (not the examples, there is another set).
They are in /test directory, but in a semi-working state - it's in my list of tasks to do, but at this moment I don't have a lot of free cycles :/ I'm happy to help if you want to take a crack at it. See #50 for details.

[selmanj]

I can look at fixing those up. I'm curious though what you mean by the VCR tapes suffering from bitrot; technically they are recorded against a specific version of the google API, so we should be safe in assuming that the API remains fairly unchanged, right? Any breakages due to bitrot are likely problems with the Google API no longer behaving correctly for a fixed version.

Regarding VCR not testing errors or changes on the backend; this is true, but again the fixed version of the API should enforce that these changes should not occur. In fact, if this does happen I would prefer to have the VCR tapes since it provides a clear example of the breaking change.

[Temikus]

@selmanj
To be clear, I'm not against having VCR tests in the future, it's just that they require quite a lot of maintenance:

• Changes in Excon often break them: Update excon dependency to version ~>0.27.0 fog#2223, [vcloud_director] Remove vcr. fog#2234
• Recording them is not entirely trivial and one needs to be very careful about recording them as to not expose private information, meaning PR's require more hand holding and review than usual.
• While unexpected behaviour from backend should not occur, it does happen: Better errors for incorrect service_accounts #45

With that being said - with current very limited resources on the project, if someone does want to help with tests, I would prefer them to finish the integration tests first so we have proper coverage against a real backend. Otherwise we may end up in a situation where we have yet another unfinished test suite on the project. I have no issues if you want to introduce VCR tests after though.

Makes sense?

[selmanj]

@Temikus I agree. Having working integration-level tests is definitely the first step.

[selmanj]

I'm going to propose the following general strategy for tests in minitest going forward:

1. Cover some (all?) of the lower-level requests with integration tests. The benefit here is that we can test our dependencies (more specifically, google-api-client and the actual google service endpoints) to make sure they haven't changed. I'm wondering what the right balance is here; a lot of the request methods are stupidly simple, and therefore testing them isn't necessarily high value. Maybe each service should have at least some request tests to catch a change to google-api-client?
2. Cover some of the expected workflows for each collection with integration tests. Here we can have end-to-end tests on the most common scenarios that a consumer might use (e.g. provision a server, add ssh key to it, put it behind a LB, de-provision, etc), which is high value. It can also serve as a high-level example to new users on how to consume the package.
3. Unit test everything else. There's lots of small bits of logic that would be tedious to test if attached to a live system; this is where unit tests come in handy. I imagine mostly these would be on the collections themselves.

I'm working on 1 and 2 above right now for compute. Personally i'd prefer to have more mock-based unit tests on some of the edge-conditions where code gets complicated as doing integration-tests can be very slow.

[Temikus]

@selmanj LGTM, let's get this ball rolling.

On 1: If we're testing requests I would go for more coverage, even if it's simple. So it's definitely important that each service is covered be it via workflow or request test.
Priority IMO should be:
P0 Cover the requests with most logic in (sadly we have those).
P1 Cover the most popular/used requests
P1 Cover some of the most popular workflows
P2 Cover everything else

[Temikus]

Integration tests are live along with unit tests \o/
Adding integrate label now runs builds in concourse with access to fog-google org.

Travis now runs unit tests as well, helping us verify that newly submitted models conform to basic lifecycle requirements.