Skip to content

Improvements to the test runners#39628

Merged
cpuguy83 merged 1 commit intomoby:masterfrom
cpuguy83:test_setup_improvements
Aug 1, 2019
Merged

Improvements to the test runners#39628
cpuguy83 merged 1 commit intomoby:masterfrom
cpuguy83:test_setup_improvements

Conversation

@cpuguy83
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 commented Jul 29, 2019 •
edited
Loading

  1. Use go list to get list of integration dirs to build. This means we
    do not need to have a valid .go in every subdirectory and also
    filters out other dirs like "bundles" which may have been created.
  2. Add option to specify custom flags for integration and
    integration-cli. This is needed so both suites can be run AND set
    custom flags... since the cli suite does not support standard go
    flags.
  3. Add options to skip an entire integration suite.

@cpuguy83 cpuguy83 requested review from thaJeztah and vdemeester July 29, 2019 23:21
@cpuguy83 cpuguy83 requested a review from tianon as a code owner July 29, 2019 23:21
@cpuguy83
Copy link
Copy Markdown
Member Author

cpuguy83 commented Jul 29, 2019

Overlooked a case where an integration dir has a .go but no test files.

16:26:02 Running /go/src/github.com/docker/docker/integration flags=-test.v -test.timeout=5m
16:26:02 env: './test.main': No such file or directory

@cpuguy83 cpuguy83 force-pushed the test_setup_improvements branch 2 times, most recently from faba62a to 8af1048 Compare July 30, 2019 17:25
@cpuguy83
Copy link
Copy Markdown
Member Author

cpuguy83 commented Jul 30, 2019

There we go, that should be good.
@thaJeztah Using go list to get only the test packages now :)

@cpuguy83 cpuguy83 force-pushed the test_setup_improvements branch from 8af1048 to 9b60d9d Compare July 30, 2019 18:50
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jul 30, 2019

Looks like the windows machines aren't happy 😞

19:03:47 ERROR: make.ps1 failed:
19:03:47 flag provided but not defined: -}}{{-
19:03:47 At line:1 char:1
19:03:47 + go list  -test -f '{{- if ne .ForTest " -}}{{- .Dir -}}{{- end -}}'   ...
19:03:47 + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
19:03:47

@cpuguy83 cpuguy83 force-pushed the test_setup_improvements branch from 9b60d9d to 0816c35 Compare July 30, 2019 21:10
@cpuguy83
Copy link
Copy Markdown
Member Author

cpuguy83 commented Jul 30, 2019

Yay powershell quoting. I think this one will work.

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jul 30, 2019

Looks like RS1 is happy now, but RS5 not

@cpuguy83
Copy link
Copy Markdown
Member Author

cpuguy83 commented Jul 30, 2019

:( They both seem to be giving the same error... works on my Windows machine.

@cpuguy83 cpuguy83 force-pushed the test_setup_improvements branch 2 times, most recently from 7bd7e48 to 5a5b690 Compare July 30, 2019 22:49
@cpuguy83
Copy link
Copy Markdown
Member Author

cpuguy83 commented Jul 30, 2019

Yay, looks like Windows is good now.

thaJeztah
thaJeztah reviewed Jul 31, 2019
View reviewed changes
thaJeztah
thaJeztah reviewed Jul 31, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question, otherwise SGTM

ping @tianon @ddebroy PTAL

@cpuguy83
Improvements to the test runners
abece9b 
1. Use `go list` to get list of integration dirs to build. This means we
   do not need to have a valid `.go` in every subdirectory and also
   filters out other dirs like "bundles" which may have been created.
2. Add option to specify custom flags for integration and
   integration-cli. This is needed so both suites can be run AND set
   custom flags... since the cli suite does not support standard go
   flags.
3. Add options to skip an entire integration suite.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 force-pushed the test_setup_improvements branch from 5a5b690 to abece9b Compare July 31, 2019 23:37
thaJeztah
thaJeztah approved these changes Aug 1, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@tianon tianon mentioned this pull request Aug 1, 2019
Closed
@tianon
Copy link
Copy Markdown
Member

tianon commented Aug 1, 2019

Seems like a much cleaner approach! 👍

LGTM

@cpuguy83
Copy link
Copy Markdown
Member Author

cpuguy83 commented Aug 1, 2019

Thanks!

@cpuguy83 cpuguy83 merged commit 4fb5e9e into moby:master Aug 1, 2019
This was referenced Aug 19, 2019
Merged
Merged
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@vdemeester vdemeester Awaiting requested review from vdemeester

@tianon tianon Awaiting requested review from tianon tianon is a code owner

Assignees

No one assigned

Labels

area/testing process/cherry-picked status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

4 participants

@cpuguy83 @thaJeztah @tianon @GordonTheTurtle