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

Fix windows tests that are failing CI on main #5916

Merged
merged 4 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 4 additions & 2 deletions component/common/loki/client/internal/marker_file_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ const (
MarkerFolderName = "remote"
MarkerFileName = "segment_marker"

MarkerFolderMode os.FileMode = 0o700
MarkerFileMode os.FileMode = 0o600
MarkerFolderMode os.FileMode = 0o700
MarkerWindowsFolderMode os.FileMode = 0o777
MarkerFileMode os.FileMode = 0o600
MarkerWindowsFileMode os.FileMode = 0o666
)

// MarkerFileHandler is a file-backed wal.Marker, that also allows one to write to the backing store as particular
Expand Down
13 changes: 11 additions & 2 deletions component/common/loki/client/internal/marker_file_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package internal
import (
"os"
"path/filepath"
"runtime"
"testing"

"github.com/go-kit/log"
Expand Down Expand Up @@ -57,10 +58,18 @@ func TestMarkerFileHandler(t *testing.T) {
// check folder first
stats, err := os.Stat(filepath.Join(dir, MarkerFolderName))
require.NoError(t, err)
require.Equal(t, MarkerFolderMode, stats.Mode().Perm())
if runtime.GOOS == "windows" {
require.Equal(t, MarkerWindowsFolderMode, stats.Mode().Perm())
} else {
require.Equal(t, MarkerFolderMode, stats.Mode().Perm())
}
// then file
stats, err = os.Stat(filepath.Join(dir, MarkerFolderName, MarkerFileName))
require.NoError(t, err)
require.Equal(t, MarkerFileMode, stats.Mode().Perm())
if runtime.GOOS == "windows" {
require.Equal(t, MarkerWindowsFileMode, stats.Mode().Perm())
} else {
require.Equal(t, MarkerFileMode, stats.Mode().Perm())
}
})
}
2 changes: 2 additions & 0 deletions integration-tests/tests/otlp-metrics/otlp_metrics_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build !windows

package main

import (
Expand Down
2 changes: 2 additions & 0 deletions integration-tests/tests/read-log-file/read_log_file_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build !windows

package main

import (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build !windows
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem with the integration tests is that they cannot run in Drone. They only run in github Actions (because it uses docker compose which is not possible in Drone). I was planning to use test-containers (see #5861) but now that we are moving away from Drone I will probably keep them in Github Actions. The reason why the tests are running is because the Windows pipeline is calling "go test" at the root of the pipeline and that triggers the integration tests (https://github.com/grafana/agent/blob/main/.drone/drone.yml#L184). They dont run when you run make test because I made a special exclusion (https://github.com/grafana/agent/blob/main/Makefile#L176). I wanted to do the same thing to fix the windows pipeline but I modified directly the yml file not knowing that it was a generated file (I just realised now) https://github.com/grafana/agent/pull/5643/files. Now Im not sure whether this fix actually work or if that's the right approach (@tpaschalis suggested also to use build tags but you need to make sure that the nothing runs in the integration tests folder)


package main

import (
Expand Down