Generate coverage statistics for integration tests #767
Conversation
efekarakus
requested review from
PettitWesley,
SoManyHs,
allisaurus and
piradeepk
|
Sample output from CodeBuild: https://gist.github.com/efekarakus/db0faa4bd48b6acb395280b8f3c23016 |
| go test -tags integ ./ecs-cli/integ/e2e/... | ||
|
|
||
| .PHONY: integ-test-run-with-coverage | ||
| integ-test-run-with-coverage: integ-test-run |
There was a problem hiding this comment.
Just for my own understanding... what this is technically doing is just processing the coverage report produced by integ-test-build?
There was a problem hiding this comment.
I'll add these explanations into our Makefile as comments:
integ-test-buildbuilds a test binary calledecs-cli.test. This binary is the same as regularecs-clibut it additionally gives coverage stats to stdout after each execution.integ-test-runruns our integration tests using theecs-cli.testbinary.integ-test-run-with-coveragefirst runsinteg-test-runand merges all the coverage files for each command in the e2e tests into 1 file. After merging them all in 1 file, we can get the full coverage stats for our tests.
Does that help?
ecs-cli/integ/cmd/compose.go
Outdated
| @@ -177,6 +176,7 @@ func testServiceHasAllRunningContainers(t *testing.T, p *Project, wantedNumOfCon | |||
|
|
|||
| // Then | |||
| lines := strings.Split(string(out), "\n") | |||
| lines = lines[:len(lines)-3] // remove coverage metadata | |||
There was a problem hiding this comment.
Dumb question: How come this is only needed in the compose command?
Minor nit: 3 feels like magic number
There was a problem hiding this comment.
Great question :) You're right this information should be redacted from all other commands. The other tests only search if substrings are contained in the command's output. So the tests never broke.
However, for ecs-cli compose service ps we have to parse the output properly to count the number of running containers so I had to modify this function.
I moved this logic to a new function Lines() in stdout.go. This way any test that cares about individual lines can use that function.
| return exec.Command(cmdPath, args...) | ||
| } | ||
|
|
||
| // createTempCoverageFile creates a coverage file for a CLI command under $TMPDIR. |
There was a problem hiding this comment.
is $TMPDIR an env set by ioutil or something we set explicitly elsewhere?
There was a problem hiding this comment.
I believe it's set by default depending on your OS.
ioutil refers to the tempDir() function in the end to figure out the location of your tmp directory.
For CodeBuild, I had to add the env variable TMPDIR that's set to /tmp. I added a new section in the README to set this variable locally if your environment doesn't set it.
| imageCommand "github.com/aws/amazon-ecs-cli/ecs-cli/modules/commands/image" | ||
| licenseCommand "github.com/aws/amazon-ecs-cli/ecs-cli/modules/commands/license" | ||
| logsCommand "github.com/aws/amazon-ecs-cli/ecs-cli/modules/commands/log" | ||
| regcredsCommand "github.com/aws/amazon-ecs-cli/ecs-cli/modules/commands/regcreds" |
There was a problem hiding this comment.
it looks like these import aliases ("regcredsCommand", etc.) are new, but references to them later in the file (e.g., lines 70-82) appear unchanged. Were these updating previously somehow? Wondering how these and their usages could be updated separately.
There was a problem hiding this comment.
I believe we didn't run goimports on this file before.
So I when I made modifications to this file, it automatically restructured our imports :/
I think it's fine to keep it as is instead of reverting to the previous look since we want to use goimports.
ecs-cli/main_test.go
Outdated
| @@ -0,0 +1,28 @@ | |||
| // +build testrunmain | |||
|
|
|||
| // Copyright 2015-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved. | |||
efekarakus
force-pushed
the
integ-tests/binary
branch
from
72e9217
to
c9118bd
Compare
|
Coverage increased to 38.4% from 29.4% after adding the ec2 task integ test. |
Description of changes:
make integ-testnow outputs our code coverage. For example, the fargate tutorial e2e test covers 29.2% of our statements.Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Documentation
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.