Use gotest.tools/gotestsum for unit and e2e tests by silvin-lubecki · Pull Request #1639 · docker/cli

silvin-lubecki · 2019-01-24T17:09:54Z

- What I did
Use gotestsum instead of go test command line to run the tests and improve the test output.

Moving from

=== RUN   TestRequiresNoArgs
--- PASS: TestRequiresNoArgs (0.00s)
=== RUN   TestRequiresMinArgs
--- PASS: TestRequiresMinArgs (0.00s)
=== RUN   TestRequiresMaxArgs
--- PASS: TestRequiresMaxArgs (0.00s)
=== RUN   TestRequiresRangeArgs
--- PASS: TestRequiresRangeArgs (0.00s)
=== RUN   TestExactArgs
--- PASS: TestExactArgs (0.00s)
PASS
ok      github.com/docker/cli/cli       (cached)
=== RUN   TestNewAPIClientFromFlags
--- PASS: TestNewAPIClientFromFlags (0.00s)
=== RUN   TestNewAPIClientFromFlagsForDefaultSchema
--- PASS: TestNewAPIClientFromFlagsForDefaultSchema (0.00s)
=== RUN   TestNewAPIClientFromFlagsWithAPIVersionFromEnv
--- PASS: TestNewAPIClientFromFlagsWithAPIVersionFromEnv (0.00s)
=== RUN   TestInitializeFromClient
=== RUN   TestInitializeFromClient/successful_ping
=== RUN   TestInitializeFromClient/failed_ping,_no_API_version
=== RUN   TestInitializeFromClient/failed_ping,_with_API_version
--- PASS: TestInitializeFromClient (0.00s)
    --- PASS: TestInitializeFromClient/successful_ping (0.00s)
    --- PASS: TestInitializeFromClient/failed_ping,_no_API_version (0.00s)
    --- PASS: TestInitializeFromClient/failed_ping,_with_API_version (0.00s)
=== RUN   TestExperimentalCLI
=== RUN   TestExperimentalCLI/default
=== RUN   TestExperimentalCLI/experimental
--- PASS: TestExperimentalCLI (0.00s)
    --- PASS: TestExperimentalCLI/default (0.00s)
    --- PASS: TestExperimentalCLI/experimental (0.00s)
=== RUN   TestGetClientWithPassword
=== RUN   TestGetClientWithPassword/successful_connect
=== RUN   TestGetClientWithPassword/password_retriever_exhausted
=== RUN   TestGetClientWithPassword/password_retriever_error
=== RUN   TestGetClientWithPassword/newClient_error
--- PASS: TestGetClientWithPassword (0.00s)
    --- PASS: TestGetClientWithPassword/successful_connect (0.00s)
    --- PASS: TestGetClientWithPassword/password_retriever_exhausted (0.00s)
    --- PASS: TestGetClientWithPassword/password_retriever_error (0.00s)
    --- PASS: TestGetClientWithPassword/newClient_error (0.00s)
=== RUN   TestNewDockerCliAndOperators
--- PASS: TestNewDockerCliAndOperators (0.00s)
=== RUN   TestOrchestratorSwitch
=== RUN   TestOrchestratorSwitch/default
=== RUN   TestOrchestratorSwitch/kubernetesConfigFile
=== RUN   TestOrchestratorSwitch/kubernetesEnv
=== RUN   TestOrchestratorSwitch/kubernetesFlag
=== RUN   TestOrchestratorSwitch/allOrchestratorFlag
=== RUN   TestOrchestratorSwitch/kubernetesContext
=== RUN   TestOrchestratorSwitch/contextOverridesConfigFile
=== RUN   TestOrchestratorSwitch/envOverridesConfigFile
=== RUN   TestOrchestratorSwitch/flagOverridesEnv


... 2000 lines later and maybe some errors hidden inside


    --- PASS: TestParseTruncateFunction/Nil_Source_Test_with_template:_{{pad_._3_3}} (0.00s)
PASS
ok      github.com/docker/cli/templates (cached)
?       github.com/docker/cli/types     [no test files]

To

✓  cli (2ms)
✓  cli/command/bundlefile
✓  cli/command/checkpoint (5ms)
✓  cli/command (1ms)
∅  cli/command/builder
∅  cli/command/commands
✓  cli/command/config (1ms)
✓  cli/command/context (1ms)
✓  cli/command/engine (1ms)
✓  cli/command/formatter (1ms)
✓  cli/command/idresolver
✓  cli/command/container (2ms)
✓  cli/command/inspect
✓  cli/command/image/build (2ms)
✓  cli/command/manifest (3ms)
✓  cli/command/network
✓  cli/command/node (3ms)
✓  cli/command/image (9ms)
✓  cli/command/plugin (10ms)
✓  cli/command/registry
✓  cli/command/service/progress
✓  cli/command/secret (1ms)
✓  cli/command/stack/formatter
✓  cli/command/stack/loader
✓  cli/command/service (3ms)
✓  cli/command/stack (1ms)
✓  cli/command/stack/swarm
✓  cli/command/swarm (1ms)
✓  cli/command/task (1ms)
✓  cli/command/system
✓  cli/command/stack/kubernetes (1ms)
∅  cli/command/stack/options
∅  cli/command/swarm/progress
✓  cli/compose/interpolation
✓  cli/compose/convert (2ms)
✓  cli/command/volume (2ms)
✓  cli/compose/loader (4ms)
✓  cli/command/trust (1ms)
✓  cli/compose/template (1ms)
✓  cli/compose/schema
∅  cli/compose/schema/data
∅  cli/compose/types
✓  cli/config/configfile
✓  cli/config (6ms)
✓  cli/connhelper/ssh
✓  cli/connhelper
✓  cli/config/credentials (1ms)
∅  cli/context
∅  cli/context/docker
✓  cli/debug
✓  cli/context/store
✓  cli/context/kubernetes
✓  cli/flags
✓  cli/manifest/store (1ms)
∅  cli/manifest/types
∅  cli/registry/client
∅  cli/streams (1ms)
✓  cmd/docker
✓  cli/trust
∅  docs/yaml
✓  internal/containerizedengine
✓  internal/licenseutils
✓  internal/pkg/containerized (1ms)
∅  internal/test
∅  internal/test/builders
∅  internal/test/environment
∅  internal/test/network
∅  internal/test/notary
∅  internal/test/output
✓  internal/versions (7ms)
∅  kubernetes
∅  kubernetes/client/clientset
∅  kubernetes/client/clientset/scheme
∅  kubernetes/client/clientset/typed/compose/v1beta1
∅  kubernetes/client/clientset/typed/compose/v1beta2
∅  kubernetes/client/informers
∅  kubernetes/client/informers/compose
∅  kubernetes/client/informers/compose/v1beta2
∅  kubernetes/client/informers/internalinterfaces
∅  kubernetes/client/listers/compose/v1beta2
∅  kubernetes/compose
∅  kubernetes/compose/clone
∅  kubernetes/compose/impersonation
✓  kubernetes/compose/v1beta1
∅  kubernetes/compose/v1beta2
∅  kubernetes/labels
✓  service/logs (1ms)
✓  templates (1ms)
∅  man
✓  opts (2ms)
∅  types

=== Skipped
=== SKIP: cli/command/container TestSplitCpArg/absolute_path_with_drive (0.01s)
    --- SKIP: TestSplitCpArg/absolute_path_with_drive (0.01s)
        cp_test.go:185: testcase.os != "" && testcase.os != runtime.GOOS

=== SKIP: cli/command/image TestNewHistoryCommandSuccess (0.00s)
    history_test.go:52: notUTCTimezone: expected output requires UTC timezone


DONE 1082 tests, 2 skipped in 53.248s

- A picture of a cute animal (not mandatory but encouraged)

codecov-io · 2019-01-24T17:17:57Z

Codecov Report

Merging #1639 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1639   +/-   ##
=======================================
  Coverage   56.11%   56.11%           
=======================================
  Files         306      306           
  Lines       20909    20909           
=======================================
  Hits        11734    11734           
  Misses       8328     8328           
  Partials      847      847

thaJeztah · 2019-01-24T17:27:27Z

I'm not sure; I think the new output is lacking information that we can use for analysis;

first of all, grepping for Fail: will no longer work
no longer to see how long tests take (which we should track btw)

I'd rather have structured (machine-readable) output, so that we can analyse the output (and perhaps have a bot show failures as a GitHub comment)

ijc · 2019-01-24T17:30:10Z

Looks like this gotestsum thing can also export junit which is what (AIUI) Jenkins consumes -- presumably that then open up a path to a bot doing something with that information.

vdemeester · 2019-01-24T17:42:31Z

Looks like this gotestsum thing can also export junit which is what (AIUI) Jenkins consumes -- presumably that then open up a path to a bot doing something with that information.

Yes it does ! 👼

* first of all, grepping for `Fail:` will no longer work

So, it still does… @silvin-lubecki's example doesn't show, but any failed test will be there (with FAIL: …).

* no longer to see how long tests take (which we should track btw)

I do agree on that for the CI 👼 I wished we would have this output for day-to-day workflow (aka contributor doing make test-unit) and the most verbose possible for the CI. Also, the report (junit, test2json) do include (or should) time and more or less, anything that we would want to track information (without having to parse the output).

dnephin · 2019-01-24T19:37:22Z

first of all, grepping for Fail: will no longer work

I think it would be more correct to say that there is no longer a need to grep for Fail because all failures will be grouped together at the end. Instead of having to search through all the text you can scroll to the end and see all the relevant details.

no longer to see how long tests take (which we should track btw)

If you really want this in the UI you can use --format standard-verbose which will give you the standard go test -v, and still give you access to the summary at the end, Junit XML, etc. I suspect test timings are more useful in the machine formats, more on that below.

I'd rather have structured (machine-readable) output, so that we can analyse the output (and perhaps have a bot show failures as a GitHub comment)

gotestsum gives you access to both. The stdout format can be set with --format and you can also write a file with junit xml so that ~~jenkins~~ CircleCI can read, and a file with the raw JSON output from go test -json that you can send to a bot, or consume from a script to find slowest tests, flaky tests, etc.

As the author of gotestsum I would be interested in hearing about any improvements you might be interested in seeing, such as other formats.

dnephin · 2019-01-24T19:38:34Z

I wished we would have this output for day-to-day workflow (aka contributor doing make test-unit) and the most verbose possible for the CI.

This is possible by changing the format using an environment variable GOTESTSUM_FORMAT.

dnephin

LGTM

I suspect you'll want to set GOTESTSUM_JUNITFILE in the circleCI config to write the junit xml to a file, and add store_test_results to the config.

That could be done in a follow up.

Also, thank you @silvin-lubecki for this PR! I'm excited about the prospect of more people benefiting from gotestsum!

vdemeester

I suspect you'll want to set GOTESTSUM_JUNITFILE in the circleCI config to write the junit xml to a file, and add store_test_results to the config.

@silvin-lubecki Yes I would like that in the PR 🙏 👼

The rest looks good to me 👼

silvin-lubecki · 2019-01-27T23:00:43Z

@dnephin the output is weird on the circle CI test step:

✓  cli (8ms)

DONE 5 tests in 3.093s
✓  cli/command (24ms)

DONE 27 tests in 15.777s
∅  cli/command/builder

DONE 0 tests in 1.049s
✓  cli/command/bundlefile (3ms)

DONE 4 tests in 0.543s
✓  cli/command/checkpoint (21ms)

DONE 7 tests in 3.500s
∅  cli/command/commands

DONE 0 tests in 9.321s
✓  cli/command/config (27ms)

DONE 18 tests in 3.537s
✓  cli/command/container (66ms)

=== Skipped
=== SKIP: cli/command/container TestSplitCpArg/absolute_path_with_drive (0.00s)
    --- SKIP: TestSplitCpArg/absolute_path_with_drive (0.00s)
        cp_test.go:185: testcase.os != "" && testcase.os != runtime.GOOS


DONE 87 tests, 1 skipped in 5.334s
✓  cli/command/context (83ms)
...

And also there is no summary at the end... Any idea?

dnephin · 2019-01-28T00:31:39Z

Ya, test runs make test-coverage, which runs ./scripts/test/unit-with-coverage $(shell go list ...), which calls script/test/unit in a loop. So gotestsum is being called multiple times.

unit-with-coverage is no longer necessary, as of some recent go release (I think it was go1.10) you can specify just -coverprofile=coverage.txt and it will do the right thing.

script/test/unit is also not very useful anymore, all it does is call a single binary.

I would git rm both scripts/test/unit and scriptes/test/unit-with-coverage and have make coverage call:

gotestsum -- -coverprofile=coverage.txt $(shell go list ./... | grep -vE '/vendor/|/e2e/')

silvin-lubecki · 2019-01-28T09:33:00Z

Much better, thank you @dnephin !

silvin-lubecki · 2019-02-01T10:14:57Z

Well, according to the circle.ci logs, the junit.xml file has been stored

Archiving the following test results
  * /work/junit.xml

Uploaded

But I honnestly don't know how to configure circle ci to use it... @dnephin ?

…ify the output. Signed-off-by: Silvin Lubecki <[email protected]>

Signed-off-by: Silvin Lubecki <[email protected]>

vdemeester

LGTM 🐯

dnephin · 2019-02-01T23:24:12Z

I think what's happening is that the docker cp is coping the xml file to /work/test-results/unit-tests instead of /work/test-results/unit-tests/junit.xml. The directory doesn't exist, so it's creating a file at that path. If you add junit.xml to the target path of the docker cp I suspect it will work.

vdemeester · 2019-02-02T07:21:37Z

@dnephin yep #1653 😝

GordonTheTurtle added the status/0-triage label Jan 24, 2019

silvin-lubecki requested review from dnephin, thaJeztah and vdemeester January 24, 2019 17:10

dnephin approved these changes Jan 24, 2019

View reviewed changes

vdemeester requested changes Jan 25, 2019

View reviewed changes

dnephin closed this Jan 28, 2019

dnephin reopened this Jan 28, 2019

silvin-lubecki force-pushed the use-gotestsum branch 4 times, most recently from bab492b to 676076a Compare February 1, 2019 09:32

silvin-lubecki added 2 commits February 1, 2019 11:16

Use gotest.tools/gotestsum binary to run unit and e2e tests and simpl…

3bd3996

…ify the output. Signed-off-by: Silvin Lubecki <[email protected]>

Better coverage output, removing unnecessary unit scripts.

277f614

Signed-off-by: Silvin Lubecki <[email protected]>

silvin-lubecki force-pushed the use-gotestsum branch 2 times, most recently from cbe9092 to a8427e8 Compare February 1, 2019 10:24

Store the junit.xml file produced while running the unit tests.

ff5a83c

Signed-off-by: Silvin Lubecki <[email protected]>

silvin-lubecki force-pushed the use-gotestsum branch from a8427e8 to ff5a83c Compare February 1, 2019 10:32

vdemeester approved these changes Feb 1, 2019

View reviewed changes

silvin-lubecki force-pushed the use-gotestsum branch from 2d4942d to ff5a83c Compare February 1, 2019 10:45

vdemeester merged commit 3b345e4 into docker:master Feb 1, 2019

GordonTheTurtle added this to the 19.03.0 milestone Feb 1, 2019

thaJeztah mentioned this pull request Jul 31, 2019

added entry for running unit tests with junit report moby/moby#39638

Merged

Conversation

silvin-lubecki commented Jan 24, 2019

Uh oh!

codecov-io commented Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

thaJeztah commented Jan 24, 2019

Uh oh!

ijc commented Jan 24, 2019

Uh oh!

vdemeester commented Jan 24, 2019

Uh oh!

dnephin commented Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnephin commented Jan 24, 2019

Uh oh!

dnephin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

silvin-lubecki commented Jan 27, 2019

Uh oh!

dnephin commented Jan 28, 2019

Uh oh!

silvin-lubecki commented Jan 28, 2019

Uh oh!

silvin-lubecki commented Feb 1, 2019

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

dnephin commented Feb 1, 2019

Uh oh!

vdemeester commented Feb 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-io commented Jan 24, 2019 •

edited

Loading

dnephin commented Jan 24, 2019 •

edited

Loading

dnephin left a comment •

edited

Loading