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.

Sign up for GitHub

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

Jump to bottom

Windows compat tasks #20

Merged
merged 9 commits into from
Apr 16, 2024
Merged

Windows compat tasks #20

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2c94dca
make binny taskfile parseable on Windows
willmurphyscode Apr 8, 2024
df14c0e
enable unit tests to run on Windows
willmurphyscode Apr 8, 2024
13182bb
skip or fix some unit tests on Windows
willmurphyscode Apr 8, 2024
8fb320e
chore: windows compatibility due to open file failures
kzantow Apr 9, 2024
21257d6
Fix and skip more unit tests
willmurphyscode Apr 12, 2024
0e0d123
prevent CRLF normalization of test fixtures
willmurphyscode Apr 12, 2024
880d8db
Enable Windows unit tests
willmurphyscode Apr 12, 2024
9e5eb89
Fix workflow actions yaml syntax
willmurphyscode Apr 12, 2024
b0f98e9
set Windows-specific coverage threshold
willmurphyscode Apr 12, 2024
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
5 changes: 3 additions & 2 deletions Taskfile.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ vars:
SNAPSHOT_DIR: snapshot
CHANGELOG: CHANGELOG.md
NEXT_VERSION: VERSION
MAKEDIR_P: 'python -c "import sys; import os; os.makedirs(sys.argv[1], exist_ok=True)"'

tasks:

Expand Down Expand Up @@ -122,12 +123,12 @@ tasks:
desc: Run all unit tests
vars:
TEST_PKGS:
sh: "go list ./... | grep -v {{ .OWNER }}/{{ .PROJECT }}/test | tr '\n' ' '"
sh: "python ./scripts/list_units.py anchore binny"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have binny, can we just have it download grep (and maybe sh, curl, and other such missing shell script utilities) instead of making the python scripts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My concern is that the utilities won't behave the same even if we make them available. For example ls lists files in PowerShell, but the output looks different than on macOS or Linux. We also see elsewhere in this PR that mkdir -p doesn't work on Windows, even though there is a mkdir on PATH in PowerShell.

In other words, if we keep using a command shell as our scripting environment, we have to worry about the compatibility of every executable the shell invokes. If we use standard library Python, we only have to worry about the cross-platform behavior of Python itself.


# unit test coverage threshold (in % coverage)
COVERAGE_THRESHOLD: 45
cmds:
- cmd: "mkdir -p {{ .TMP_DIR }}"
- cmd: "{{ .MAKEDIR_P }} {{ .TMP_DIR }}"
silent: true
- "go test -coverprofile {{ .TMP_DIR }}/unit-coverage-details.txt {{ .TEST_PKGS }}"
- cmd: ".github/scripts/coverage.py {{ .COVERAGE_THRESHOLD }} {{ .TMP_DIR }}/unit-coverage-details.txt"
Expand Down
18 changes: 18 additions & 0 deletions scripts/list_units.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import subprocess
import sys

def go_list_exclude_pattern(owner, project):
exclude_pattern = f"{owner}/{project}/test"

result = subprocess.run(["go", "list", "./..."], stdout=subprocess.PIPE, text=True, check=True)

filtered_lines = [line for line in result.stdout.splitlines() if exclude_pattern not in line]

joined_output = ' '.join(filtered_lines)

return joined_output

owner = sys.argv[1]
project = sys.argv[2]
output = go_list_exclude_pattern(owner, project)
print(output)
4 changes: 4 additions & 0 deletions store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package binny
import (
"os"
"path/filepath"
"runtime"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -104,6 +105,9 @@ func TestStore_GetByName(t *testing.T) {
}

func TestStore_Entries(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("test fixtures have different sha256 digests on Windows due to linbreak conversion")
}
tests := []struct {
name string
storeRoot string
Expand Down
3 changes: 3 additions & 0 deletions tool/goinstall/installer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package goinstall

import (
"fmt"
"os"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -108,6 +110,7 @@ func TestInstaller_InstallTo(t *testing.T) {
i.goInstallRunner = tt.fields.goInstallRunner

got, err := i.InstallTo(tt.args.version, tt.args.destDir)
got = strings.ReplaceAll(got, string(os.PathSeparator), "/")
Copy link
Contributor

Choose a reason for hiding this comment

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

This test fails for me because there is no sh by default on windows, regardless of the path. I wonder if we could do the same thing that the taskfile is doing and use https://github.com/mvdan/sh for these...

if !tt.wantErr(t, err, fmt.Sprintf("InstallTo(%v, %v)", tt.args.version, tt.args.destDir)) {
return
}
Expand Down
Loading