We have no functional tests, so we can't enforce expectations about things like which commands should be present and how they should behave.
This is necessary during refactoring, to be sure we aren't regressing.
Create a test base class which can create or mock an app for us.
For some tests, use Symfony\Component\Console\Tester\CommandTester to run commands and capture output.
For some tests, require dependencies on mock 'hard' resources such as loading a fixture image.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | pullTest.patch | 1.84 KB | jthorson |
| #3 | 2676854_3.patch | 3.3 KB | mile23 |
Comments
Comment #2
mile23This issue is immediately postponed on #2638466: Add a php container to make the codebase easier to test., so we can build our test infrastructure on the new service container.
Comment #3
mile23...Tho of course there's already a patch. :-)
It depends on the service providers being defined in #2638466: Add a php container to make the codebase easier to test. so it might need a reroll or scrap and restart later.
Comment #4
mile23The container is in, let's rock.
Comment #6
mile23This is the patch in #3, plus a little shuffling to put
ConsoleCommandProviderin the proper place.If you run it you'll see that we lost the travis command in there somewhere. It probably doesn't have a command service.
So this issue is about using the CommandTester class to run commands and checking expectations about their output. You can see an example of this in
StatusCommandTest, which was an easy test to write because the status command doesn't do much. :-)CommandTester just captures the output of the command run and then you can examine the text that came back. You can also examine the state of the command itself because you still have an instance of it.
Testing gets more complicated with, eg, the run command which has a lot of dependencies.
Comment #7
jthorson commentedHere's where I wanted to sneak some of the changes in before testing.
If CommandTester could capture the JobResults object (part of #2655718: Create the BuildResults), we would then have a per-build step status reported back from each individual step executed within something as complicated as the RunCommand ... and given various configurations, we could iterate over these results to confirm the expected behaviour. While admittedly, this doesn't give us coverage of the JobResults calculation itself, it would give us a significantly larger chunk of coverage than what we have today ... and with much lighter test logic!
Comment #9
mile23Functional tests.
Of command output given input.
That's what this issue is. :-)
Let's now say that the JobResults object is a service parameter set by the command.
This means that for testing you'd mock a service, like docker or whatever, put it in the container, create a command, and inject the service. Then look at the job.results service parameter when it's done.
Assert your expectations, and bob's your uncle.
But not in this issue. :-)
Comment #10
jthorson commentedLet's rock!
Comment #11
mile23There is only one functional test here.
Comment #12
jthorson commentedI took this to mean that this issue is about getting the framework in place, not completing the full list of functional tests (which could take weeks, depending on how far we want to take it).
Committing this allows us to incrementally add functional tests - as opposed to building up a large suite of them, but not adding any until we've built them all ... It would make sense to me to put in what we've got here, and isolate debates over the actual implementation of various functional tests in their own issues.
Comment #13
jthorson commentedSo ... something like this?
- drupalci/scratch is a new empty container (Copy of the official 'scratch' image; but we can't use that directly as 'scratch' is a reserved word since Docker 1.8.1.
- This doesn't actually provide coverage of execution of the 'docker pull' command itself, but I'm assuming that if we have a valid ImageManager, and attempt to 'pull' after verifying the proper output text, we will either get a pull, or an API exception back from Docker. I don't see a way of explicitly validating the pull succeeded, as the progress bar output doesn't show up in the CommandTester->getDisplay() output; but I assume that the testing framework will capture any errors which creep up there.
Comment #14
jthorson commentedOoops ... IDE "helping" is hurting. Clean patch here. :)
Comment #16
mile23These tests only have to show that when we change the output system, we still have the same output. We could make it super-perfect, or not that perfect at all. The above commit isn't all that perfect. :-)
And: A little bit like that, yes. But instead of requiring that we hit docker, we can mock the docker service with predictable results for the pull command. My test in the commit is bad and just uses a bad image name, so there's a @todo about that.
Also moved registering
DockerServiceProviderto insideDrupalCIServiceProviderso it's always there.Comment #17
mile23Remaining:
Comment #18
mile23Comment #19
MixologicFor the particular example jthorson posted above, we're wanting a functional test that proves that swapping out the docker library doesn't break anything else, so we do want a docker mock for other tests, but we want an end to end that includes the real library.
We probably want child issues and call this a meta, so that we dont get confused about different aims (being able to prove that the output changes are okay, vs being able to prove that the docker changes are okay).
Comment #20
mile23OK so then we also need a TESTING.md to explain that you need to be running in the VM or whatever, with all the dependencies.
Comment #21
MixologicHaving a doc explaining the nuance does sound like a good idea. We want functional regression tests, and many of those will probably need to be running in the vagrant VM. and we want unit tests, which shouldn't have any sort of dependency like that. Im currently working on a 'testing goals/policy' doc, detailing where we'd like to end up and how we'd like to get there.
Comment #22
mile23OK, it's a plan. :-)
Comment #23
mile23Added child issue: #2679673: Create minimal functional tests of command output to test Output refactoring
Comment #24
mile23Moved branch
2676854-write-functional-teststo2679673-write-output-testsComment #25
MixologicComment #26
mile23This is really a duplicate of #2680565: [meta] Create functional tests of run command output to test production essentials