Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: test-unit job matrix for win/macos/ubuntu #2206

Merged
merged 3 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 82 additions & 12 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,13 @@ env:
REPO_SLUG: "docker/buildx-bin"
DESTDIR: "./bin"
TEST_CACHE_SCOPE: "test"
TESTFLAGS: "-v --parallel=6 --timeout=30m"
GOTESTSUM_FORMAT: "standard-verbose"
GO_VERSION: "1.21.6"
GOTESTSUM_VERSION: "v1.9.0" # same as one in Dockerfile

jobs:
prepare-test:
prepare-test-integration:
runs-on: ubuntu-22.04
steps:
-
Expand All @@ -51,14 +55,12 @@ jobs:
*.cache-from=type=gha,scope=${{ env.TEST_CACHE_SCOPE }}
*.cache-to=type=gha,scope=${{ env.TEST_CACHE_SCOPE }}

test:
test-integration:
runs-on: ubuntu-22.04
needs:
- prepare-test
- prepare-test-integration
env:
TESTFLAGS: "-v --parallel=6 --timeout=30m"
TESTFLAGS_DOCKER: "-v --parallel=1 --timeout=30m"
GOTESTSUM_FORMAT: "standard-verbose"
TEST_IMAGE_BUILD: "0"
TEST_IMAGE_ID: "buildx-tests"
strategy:
Expand All @@ -71,9 +73,6 @@ jobs:
- remote
pkg:
- ./tests
include:
- pkg: ./...
skip-integration-tests: 1
Comment on lines -74 to -76
Copy link
Member Author

@crazy-max crazy-max Jan 25, 2024

Choose a reason for hiding this comment

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

@jedevc I removed this one as it's already handled in test-unit job through ubuntu-latest otherwise we have dup testing. Let me know if that sounds good.

steps:
-
name: Checkout
Expand Down Expand Up @@ -101,19 +100,19 @@ jobs:
-
name: Test
run: |
export TEST_REPORT_SUFFIX=-${{ github.job }}-$(echo "${{ matrix.pkg }}-${{ matrix.skip-integration-tests }}-${{ matrix.worker }}" | tr -dc '[:alnum:]-\n\r' | tr '[:upper:]' '[:lower:]')
export TEST_REPORT_SUFFIX=-${{ github.job }}-$(echo "${{ matrix.pkg }}-${{ matrix.worker }}" | tr -dc '[:alnum:]-\n\r' | tr '[:upper:]' '[:lower:]')
./hack/test
env:
TEST_DOCKERD: "${{ startsWith(matrix.worker, 'docker') && '1' || '0' }}"
TESTFLAGS: "${{ (matrix.worker == 'docker' || matrix.worker == 'docker\\+containerd') && env.TESTFLAGS_DOCKER || env.TESTFLAGS }} --run=//worker=${{ matrix.worker }}$"
TESTPKGS: "${{ matrix.pkg }}"
SKIP_INTEGRATION_TESTS: "${{ matrix.skip-integration-tests }}"
-
name: Send to Codecov
if: always()
uses: codecov/codecov-action@v3
with:
directory: ./bin/testreports
flags: integration
Copy link
Member Author

Choose a reason for hiding this comment

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

Added some flags for codecov so we have a clear distinction between unit and integration tests

-
name: Generate annotations
if: always()
Expand All @@ -128,6 +127,75 @@ jobs:
name: test-reports
path: ./bin/testreports

test-unit:
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os:
- ubuntu-22.04
- macos-12
- windows-2022
env:
SKIP_INTEGRATION_TESTS: 1
steps:
-
name: Checkout
uses: actions/checkout@v4
-
name: Set up Go
uses: actions/setup-go@v4
with:
go-version: "${{ env.GO_VERSION }}"
-
name: Prepare
run: |
testreportsBaseDir=./bin/testreports
testreportsDir=$testreportsBaseDir/unit-${{ matrix.os }}
echo "TESTREPORTS_BASEDIR=$testreportsBaseDir" >> $GITHUB_ENV
echo "TESTREPORTS_DIR=$testreportsDir" >> $GITHUB_ENV
mkdir -p $testreportsDir
shell: bash
-
name: Install gotestsum
run: |
go install gotest.tools/gotestsum@${{ env.GOTESTSUM_VERSION }}
-
name: Test
env:
TMPDIR: ${{ runner.temp }}
run: |
gotestsum \
--jsonfile="${{ env.TESTREPORTS_DIR }}/go-test-report.json" \
--junitfile="${{ env.TESTREPORTS_DIR }}/junit-report.xml" \
--packages="./..." \
-- \
"-mod=vendor" \
"-coverprofile" "${{ env.TESTREPORTS_DIR }}/coverage.txt" \
"-covermode" "atomic" ${{ env.TESTFLAGS }}
shell: bash
-
name: Send to Codecov
if: always()
uses: codecov/codecov-action@v3
with:
directory: ${{ env.TESTREPORTS_DIR }}
env_vars: RUNNER_OS
flags: unit
-
name: Generate annotations
if: always()
uses: crazy-max/.github/.github/actions/gotest-annotations@1a64ea6d01db9a48aa61954cb20e265782c167d9
with:
directory: ${{ env.TESTREPORTS_DIR }}
-
name: Upload test reports
if: always()
uses: actions/upload-artifact@v3
with:
name: test-reports
path: ${{ env.TESTREPORTS_BASEDIR }}

prepare-binaries:
runs-on: ubuntu-22.04
outputs:
Expand Down Expand Up @@ -192,7 +260,8 @@ jobs:
bin-image:
runs-on: ubuntu-22.04
needs:
- test
- test-integration
- test-unit
if: ${{ github.event_name != 'pull_request' && github.repository == 'docker/buildx' }}
steps:
-
Expand Down Expand Up @@ -244,7 +313,8 @@ jobs:
release:
runs-on: ubuntu-22.04
needs:
- test
- test-integration
- test-unit
- binaries
steps:
-
Expand Down
19 changes: 15 additions & 4 deletions build/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
if filepath.IsAbs(contextPath) {
wd = contextPath
} else {
cwd, _ := os.Getwd()
wd, _ = filepath.Abs(filepath.Join(cwd, contextPath))
wd, _ = filepath.Abs(filepath.Join(getWd(), contextPath))
}
wd = gitutil.SanitizePath(wd)

gitc, err := gitutil.New(gitutil.WithContext(ctx), gitutil.WithWorkingDir(wd))
if err != nil {
Expand Down Expand Up @@ -104,8 +104,7 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
dockerfilePath = filepath.Join(wd, "Dockerfile")
}
if !filepath.IsAbs(dockerfilePath) {
cwd, _ := os.Getwd()
dockerfilePath = filepath.Join(cwd, dockerfilePath)
dockerfilePath = filepath.Join(getWd(), dockerfilePath)
}
if r, err := filepath.Rel(root, dockerfilePath); err == nil && !strings.HasPrefix(r, "..") {
res["label:"+DockerfileLabel] = r
Expand All @@ -125,9 +124,21 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
if err != nil {
continue
}
if lp, err := getLongPathName(dir); err == nil {
dir = lp
}
dir = gitutil.SanitizePath(dir)
if r, err := filepath.Rel(root, dir); err == nil && !strings.HasPrefix(r, "..") {
so.FrontendAttrs["vcs:localdir:"+k] = r
}
}
}, nil
}

func getWd() string {
wd, _ := os.Getwd()
if lp, err := getLongPathName(wd); err == nil {
return lp
}
return wd
}
9 changes: 9 additions & 0 deletions build/git_unix.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//go:build !windows
// +build !windows

package build

// getLongPathName is a no-op on non-Windows platforms.
func getLongPathName(path string) (string, error) {
return path, nil
}
26 changes: 26 additions & 0 deletions build/git_windows.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package build
Copy link
Member Author

@crazy-max crazy-max Jan 25, 2024

Choose a reason for hiding this comment

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

Not sure if we should put this in this package or util one. Maybe on buildkit as it might be useful for Windows support (cc @TBBle @gabriel-samfira)


import "golang.org/x/sys/windows"

// getLongPathName converts Windows short pathnames to full pathnames.
// For example C:\Users\ADMIN~1 --> C:\Users\Administrator.
func getLongPathName(path string) (string, error) {
// See https://groups.google.com/forum/#!topic/golang-dev/1tufzkruoTg
p, err := windows.UTF16FromString(path)
if err != nil {
return "", err
}
b := p // GetLongPathName says we can reuse buffer
n, err := windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
if err != nil {
return "", err
}
if n > uint32(len(b)) {
b = make([]uint16, n)
_, err = windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
if err != nil {
return "", err
}
}
return windows.UTF16ToString(b), nil
}
Comment on lines +5 to +26
Copy link
Member Author

Choose a reason for hiding this comment

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

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ require (
go.opentelemetry.io/otel/trace v1.19.0
golang.org/x/mod v0.11.0
golang.org/x/sync v0.3.0
golang.org/x/sys v0.15.0
golang.org/x/term v0.15.0
google.golang.org/grpc v1.58.3
gopkg.in/yaml.v3 v3.0.1
Expand Down Expand Up @@ -152,7 +153,6 @@ require (
golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 // indirect
golang.org/x/net v0.17.0 // indirect
golang.org/x/oauth2 v0.10.0 // indirect
golang.org/x/sys v0.15.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.3.0 // indirect
golang.org/x/tools v0.10.0 // indirect
Expand Down
2 changes: 1 addition & 1 deletion util/gitutil/gitutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (c *Git) RootDir() (string, error) {
if err != nil {
return "", err
}
return sanitizePath(root), nil
return SanitizePath(root), nil
}

func (c *Git) GitDir() (string, error) {
Expand Down
2 changes: 1 addition & 1 deletion util/gitutil/path_unix.go → util/gitutil/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func gitPath(wd string) (string, error) {

var windowsPathRegex = regexp.MustCompile(`^[A-Za-z]:[\\/].*$`)

func sanitizePath(path string) string {
func SanitizePath(path string) string {
// If we're running in WSL, we need to convert Windows paths to Unix paths.
// This is because the git binary can be invoked through `git.exe` and
// therefore returns Windows paths.
Expand Down
4 changes: 2 additions & 2 deletions util/gitutil/path_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import (
)

func TestSanitizePathUnix(t *testing.T) {
assert.Equal(t, "/home/foobar", sanitizePath("/home/foobar"))
assert.Equal(t, "/home/foobar", SanitizePath("/home/foobar"))
}

func TestSanitizePathWSL(t *testing.T) {
t.Setenv("WSL_DISTRO_NAME", "Ubuntu")
assert.Equal(t, "/mnt/c/Users/foobar", sanitizePath("C:\\Users\\foobar"))
assert.Equal(t, "/mnt/c/Users/foobar", SanitizePath("C:\\Users\\foobar"))
}
2 changes: 1 addition & 1 deletion util/gitutil/path_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ func gitPath(wd string) (string, error) {
return exec.LookPath("git.exe")
}

func sanitizePath(path string) string {
func SanitizePath(path string) string {
return filepath.ToSlash(filepath.Clean(path))
}
2 changes: 1 addition & 1 deletion util/gitutil/path_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ import (
)

func TestSanitizePathWindows(t *testing.T) {
assert.Equal(t, "C:\\Users\\foobar", sanitizePath("C:/Users/foobar"))
assert.Equal(t, "C:\\Users\\foobar", SanitizePath("C:/Users/foobar"))
}