Skip to content

Add appveyor setup to build and unit test - #905

Merged
thaJeztah merged 3 commits into
docker:masterfrom
vdemeester:appveyor-setup
Mar 8, 2018
Merged

thaJeztah merged 3 commits into
docker:masterfrom
vdemeester:appveyor-setup

Conversation

@vdemeester

@vdemeester vdemeester commented Feb 27, 2018 •

Copy link
Copy Markdown
Collaborator

The next step is to run e2e tests on windows too.

Adds a make.ps1 powershell script to make it easy to compile and test.

.\scripts\make.ps1 -Binary
INFO: make.ps1 starting at 03/01/2018 14:37:28
INFO: Building...

 ________   ____  __.
 \_____  \ |    |/ _|
 /   |   \|      <
 /    |    \    |  \
 \_______  /____|__ \
         \/        \/

INFO: make.ps1 ended at 03/01/2018 14:37:30

.\scripts\make.ps1 -TestUnit

Related to #457

Signed-off-by: Vincent Demeester vincent@sbr.pm

@vdemeester

Copy link
Copy Markdown
Collaborator Author

Now the main question, is how to make appveyor build this pull-request… 🤔

@codecov-io

codecov-io commented Feb 27, 2018 •

Copy link
Copy Markdown

Codecov Report

Merging #905 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #905      +/-   ##
==========================================
- Coverage   53.55%   53.54%   -0.02%     
==========================================
  Files         262      262              
  Lines       16602    16602              
==========================================
- Hits         8891     8889       -2     
- Misses       7121     7122       +1     
- Partials      590      591       +1

@dnephin

dnephin commented Feb 27, 2018

Copy link
Copy Markdown
Contributor

I created https://ci.appveyor.com/project/docker/cli but I think someone may still need to add the github webhook

Comment thread appveyor.yml Outdated

test_script:
- go build github.com/docker/cli/cmd/docker
- for /f "" %%G in ('go list github.com/docker/cli/...') do ( go test %%G & IF ERRORLEVEL == 1 EXIT 1) No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dnephin

dnephin commented Feb 27, 2018 •

Copy link
Copy Markdown
Contributor

The hook is added, so this is running now!

(I rebased and force pushed to trigger the build)

@vdemeester

Copy link
Copy Markdown
Collaborator Author

I'll add commits from #906 here I think (to make the first run green 👼)

Comment thread appveyor.yml Outdated
- rmdir c:\go /s /q
- appveyor DownloadFile https://storage.googleapis.com/golang/go%GOVERSION%.windows-amd64.msi
- msiexec /i go%GOVERSION%.windows-amd64.msi /q
- set Path=c:\go\bin;c:\gopath\bin;C:\Program Files (x86)\Bazaar\;C:\Program Files\Mercurial\%Path%

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this line is necessary. At least no the Bazaar/Mercurial parts

Comment thread appveyor.yml Outdated

test_script:
- go build github.com/docker/cli/cmd/docker
- for /f "" %%G in ('go list github.com/docker/cli/...') do ( go test %%G & IF ERRORLEVEL == 1 EXIT 1) No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vdemeester
vdemeester force-pushed the appveyor-setup branch 4 times, most recently from 6c024d4 to 6501571 Compare March 1, 2018 10:50
@vdemeester vdemeester changed the title Add appveyor setup to build and unit test [wip] Add appveyor setup to build and unit test Mar 1, 2018
@vdemeester
vdemeester force-pushed the appveyor-setup branch 7 times, most recently from 6e32e06 to 5a17a97 Compare March 1, 2018 14:31
@vdemeester vdemeester changed the title [wip] Add appveyor setup to build and unit test Add appveyor setup to build and unit test Mar 1, 2018
@dnephin

dnephin commented Mar 6, 2018

Copy link
Copy Markdown
Contributor

Sorry about conflicts from the gotestyourself/assert PR.

Comment thread cli/command/trust/key_load_test.go Outdated
}

func TestLoadKeyFromPath(t *testing.T) {
skip.If(t, func() bool { return runtime.GOOS == "windows" })

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The func() bool {} can be removed here, and below.

Comment thread cli/compose/loader/full-struct_test.go Outdated
}

func fullExampleYAML(workingDir, homeDir string) string {
return fmt.Sprintf(string(`version: "3.6"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The string() cast is unnecessary here

Comment thread cli/registry/client/client_test.go Outdated
@@ -0,0 +1 @@
package client

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

empty file can be removed

Comment thread scripts/make.ps1 Outdated

Usage Examples (run from repo root):
"hack\make.ps1 -Client" to build docker.exe client 64-bit binary (remote repo)
"hack\make.ps1 -TestUnit" to run unit tests

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

old comments, wrong path.

Comment thread scripts/make.ps1 Outdated
@@ -0,0 +1,197 @@
<#
.NOTES
Author: @vdemeester

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

files shouldnt have authors, all the maintainers are responsible

Comment thread scripts/make.ps1 Outdated
if (-not (Test-Path ".\.git")) {
# If we don't have a .git directory, but we do have the environment
# variable DOCKER_GITCOMMIT set, that can override it.
if ($env:DOCKER_GITCOMMIT.Length -eq 0) {

@justincormack justincormack Mar 6, 2018 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can use $env:DOCKER_GITCOMMIT -eq $null it is more idiomatic for powershell

@justincormack

Copy link
Copy Markdown
Contributor

Needs a rebase

@dnephin

dnephin commented Mar 6, 2018 •

Copy link
Copy Markdown
Contributor

Rebased, and fixed most of my comments (didn't touch the powershell)

Adds a `make.ps1` powershell script to make it easy to compile and test.

```
.\scripts\make.ps1 -Binary
INFO: make.ps1 starting at 03/01/2018 14:37:28
INFO: Building...

 ________   ____  __.
 \_____  \ |    |/ _|
 /   |   \|      <
 /    |    \    |  \
 \_______  /____|__ \
         \/        \/

INFO: make.ps1 ended at 03/01/2018 14:37:30

.\scripts\make.ps1 -TestUnit
```

The next step is to run e2e tests on windows too.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Some of them are skipped for now (because the feature is not supported
or needs more work), some of them are fixed.

Signed-off-by: Vincent Demeester <vincent@sbr.pm>
Signed-off-by: Vincent Demeester <vincent@sbr.pm>
@vdemeester

Copy link
Copy Markdown
Collaborator Author

@justincormack @dnephin rebased and updated the powershell 👼

@dnephin dnephin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment thread appveyor.yml

install:
- rmdir c:\go /s /q
- appveyor DownloadFile https://storage.googleapis.com/golang/go%GOVERSION%.windows-amd64.msi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't that be prefixed with appveyor-retry just to be on the safe side ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@mat007 noted, thanks 😛 We never had any problem so far with that, so I'm fine merging like this. But if somehow it turns out we need to be more on the safe side, we'll use appveyor-retry then 😉

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants