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

Fix tests for Windows - part 1 #8414

Merged
merged 19 commits into from
Nov 23, 2020
Merged

Fix tests for Windows - part 1 #8414

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4525969
Fix tests for Windows - part 1
Nov 15, 2020
0068866
Fix tests for Windows - part 1
Nov 16, 2020
6fa8b47
Fix tests for Windows - part 1
Nov 16, 2020
275c49f
Fix tests for Windows - part 1
Nov 16, 2020
ef9c9d8
Fix tests for Windows - part 1
Nov 16, 2020
ef0a8c1
Fix tests for Windows - part 1 (do not install mingw)
Nov 16, 2020
3f20a2d
Fix tests for Windows - part 1 (addressing comments)
Nov 16, 2020
d06d847
Fix tests for Windows - part 1 (addressing comments)
Nov 16, 2020
4883fd1
Fix tests for Windows - part 1 (unblock tail_test)
Nov 17, 2020
5ac45a7
Fix tests for Windows - part 1 (unblock passenger_test)
Nov 17, 2020
0fc291d
Fix tests for Windows - part 1 (fix few globpath tests, but still glo…
Nov 21, 2020
3da8de9
Fix tests for Windows - part 1 (disabling tests which don't work, put…
Nov 22, 2020
0672287
Fix tests for Windows - part 1 (checking race detector for Windows)
Nov 22, 2020
f38e45c
Fix tests for Windows - part 1 (revert race detector for Windows - do…
Nov 22, 2020
47bc7f0
Fix tests for Windows - part 1 (enable tests for syslog plugin)
Nov 22, 2020
4bb1dc0
Fix tests for Windows - part 1 (enable tests for reverse_dns plugin -…
Nov 22, 2020
7fa1643
Fix tests for Windows - part 1 (enable tests for ifname plugin - issu…
Nov 22, 2020
15b1ac3
Fix tests for Windows - part 1 (enable tests for execd plugin)
Nov 22, 2020
a8b5806
Merge remote-tracking branch 'upstream/master' into fix_tests_for_win…
Nov 23, 2020
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
1 change: 1 addition & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ README.md merge=union
go.sum merge=union
plugins/inputs/all/all.go merge=union
plugins/outputs/all/all.go merge=union
**/testdata/** test eol=lf
7 changes: 1 addition & 6 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,7 @@ fmtcheck:

.PHONY: test-windows
test-windows:
go test -short $(race_detector) ./plugins/inputs/ping/...
go test -short $(race_detector) ./plugins/inputs/win_perf_counters/...
go test -short $(race_detector) ./plugins/inputs/win_services/...
go test -short $(race_detector) ./plugins/inputs/procstat/...
go test -short $(race_detector) ./plugins/inputs/ntpq/...
go test -short $(race_detector) ./plugins/processors/port_name/...
go test -short ./...
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason we dropped the race detector?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Build with race detector temporary enabled: https://ci.appveyor.com/project/influx/telegraf/builds/36450543

go test -short -race ./...
# runtime/cgo
exec: "gcc": executable file not found in %PATH%
?   	github.com/influxdata/telegraf	[no test files]
FAIL	github.com/influxdata/telegraf/agent [build failed]
FAIL	github.com/influxdata/telegraf/config [build failed]

Probably related to golang/go#27089

Few days ago I tried with mingw installed https://ci.appveyor.com/project/influx/telegraf/builds/36325329

make test-windows
go test -short -race ./...
go build runtime/cgo: C:\go\pkg\tool\windows_amd64\cgo.exe: exit status 2
?   	github.com/influxdata/telegraf	[no test files]
FAIL	github.com/influxdata/telegraf/agent [build failed]
FAIL	github.com/influxdata/telegraf/config [build failed]

But it works fine on my local Windows.

@ssoroka if you have any idea how to configure CI for Windows to run tests with -race let me know.


.PHONY: vet
vet:
Expand Down
4 changes: 2 additions & 2 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ clone_folder: C:\gopath\src\github.com\influxdata\telegraf
environment:
GOPATH: C:\gopath

stack: go 1.14
stack: go 1.15

platform: x64

install:
- choco install make
- choco install make mingw
- cd "%GOPATH%\src\github.com\influxdata\telegraf"
- git config --system core.longpaths true
- go version
Expand Down
3 changes: 2 additions & 1 deletion config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package config

import (
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -201,7 +202,7 @@ func TestConfig_LoadSpecialTypes(t *testing.T) {
// Tests telegraf size parsing.
assert.Equal(t, internal.Size{Size: 1024 * 1024}, inputHTTPListener.MaxBodySize)
// Tests toml multiline basic strings.
assert.Equal(t, "/path/to/my/cert\n", inputHTTPListener.TLSCert)
assert.Equal(t, "/path/to/my/cert", strings.TrimRight(inputHTTPListener.TLSCert, "\r\n"))
}

func TestConfig_FieldNotDefined(t *testing.T) {
Expand Down
8 changes: 0 additions & 8 deletions config/testdata/telegraf-agent.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,6 @@
# If no servers are specified, then 127.0.0.1 is used as the host and 4020 as the port.
servers = ["127.0.0.1:4021"]

# Read metrics from local Lustre service on OST, MDS
[[inputs.lustre2]]
# An array of /proc globs to search for Lustre stats
# If not specified, the default will work on Lustre 2.5.x
#
# ost_procfiles = ["/proc/fs/lustre/obdfilter/*/stats", "/proc/fs/lustre/osd-ldiskfs/*/stats"]
# mds_procfiles = ["/proc/fs/lustre/mdt/*/md_stats"]

# Read metrics about memory usage
[[inputs.mem]]
# no configuration
Expand Down
4 changes: 4 additions & 0 deletions internal/globpath/globpath_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// +build !windows

// TODO: should be enabled for Windows when Glob related issues for Windows are fixed
Copy link
Contributor

@ssoroka ssoroka Nov 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

any idea what these failures are?
[edit] just curious. feel free to ignore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ssoroka Yes, super asterisk doesn't work for Windows, probably because there is bug in internal/globpath/globpath.go.

I put more information here: #6248 (comment)

Fixing that issue in internal/globpath/globpath.go will enable super asterisk for Windows for these plugins:

  • file
  • filecount
  • filestat
  • logparser
  • phpfpm
  • tail


package globpath

import (
Expand Down
18 changes: 9 additions & 9 deletions plugins/inputs/bcache/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ cache_readaheads
Using this configuration:

```toml
[bcache]
# Bcache sets path
# If not specified, then default is:
# bcachePath = "/sys/fs/bcache"
#
# By default, telegraf gather stats for all bcache devices
# Setting devices will restrict the stats to the specified
# bcache devices.
# bcacheDevs = ["bcache0", ...]
[[inputs.bcache]]
## Bcache sets path
## If not specified, then default is:
bcachePath = "/sys/fs/bcache"

## By default, Telegraf gather stats for all bcache devices
## Setting devices will restrict the stats to the specified
## bcache devices.
bcacheDevs = ["bcache0"]
```

When run with:
Expand Down
4 changes: 3 additions & 1 deletion plugins/inputs/bcache/bcache.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build !windows

package bcache

import (
Expand All @@ -22,7 +24,7 @@ var sampleConfig = `
## If not specified, then default is:
bcachePath = "/sys/fs/bcache"

## By default, telegraf gather stats for all bcache devices
## By default, Telegraf gather stats for all bcache devices
## Setting devices will restrict the stats to the specified
## bcache devices.
bcacheDevs = ["bcache0"]
Expand Down
2 changes: 2 additions & 0 deletions plugins/inputs/bcache/bcache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build !windows

package bcache

import (
Expand Down
3 changes: 3 additions & 0 deletions plugins/inputs/bcache/bcache_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// +build windows

package bcache
2 changes: 2 additions & 0 deletions plugins/inputs/ceph/ceph_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// +build !windows

package ceph

import (
Expand Down
11 changes: 6 additions & 5 deletions plugins/inputs/disk/disk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package disk

import (
"fmt"
"os"
"testing"

Expand Down Expand Up @@ -74,13 +75,13 @@ func TestDiskUsage(t *testing.T) {
assert.Equal(t, expectedAllDiskMetrics, numDiskMetrics)

tags1 := map[string]string{
"path": "/",
"path": string(os.PathSeparator),
"fstype": "ext4",
"device": "sda",
"mode": "ro",
}
tags2 := map[string]string{
"path": "/home",
"path": fmt.Sprintf("%chome", os.PathSeparator),
"fstype": "ext4",
"device": "sdb",
"mode": "rw",
Expand Down Expand Up @@ -144,7 +145,7 @@ func TestDiskUsageHostMountPrefix(t *testing.T) {
},
},
expectedTags: map[string]string{
"path": "/",
"path": string(os.PathSeparator),
"device": "sda",
"fstype": "ext4",
"mode": "ro",
Expand Down Expand Up @@ -177,7 +178,7 @@ func TestDiskUsageHostMountPrefix(t *testing.T) {
},
hostMountPrefix: "/hostfs",
expectedTags: map[string]string{
"path": "/var",
"path": fmt.Sprintf("%cvar", os.PathSeparator),
"device": "sda",
"fstype": "ext4",
"mode": "ro",
Expand Down Expand Up @@ -210,7 +211,7 @@ func TestDiskUsageHostMountPrefix(t *testing.T) {
},
hostMountPrefix: "/hostfs",
expectedTags: map[string]string{
"path": "/",
"path": string(os.PathSeparator),
"device": "sda",
"fstype": "ext4",
"mode": "ro",
Expand Down
4 changes: 4 additions & 0 deletions plugins/inputs/exec/exec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// +build !windows

// TODO: should be enabled for Windows when Glob related issues for Windows are fixed

package exec

import (
Expand Down
4 changes: 4 additions & 0 deletions plugins/inputs/file/file_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// +build !windows

// TODO: should be enabled for Windows when Glob related issues for Windows are fixed

package file

import (
Expand Down
4 changes: 4 additions & 0 deletions plugins/inputs/filecount/filecount_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// +build !windows

// TODO: should be enabled for Windows when Glob related issues for Windows are fixed

package filecount

import (
Expand Down
4 changes: 4 additions & 0 deletions plugins/inputs/filecount/filesystem_helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// +build !windows

// TODO: should be enabled for Windows when Glob related issues for Windows are fixed

package filecount

import (
Expand Down
24 changes: 17 additions & 7 deletions plugins/inputs/filestat/filestat_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// +build !windows

// TODO: should be enabled for Windows when Glob related issues for Windows are fixed

package filestat

import (
"os"
"runtime"
"strings"
"testing"
Expand All @@ -18,7 +23,7 @@ func TestGatherNoMd5(t *testing.T) {
fs.Files = []string{
dir + "log1.log",
dir + "log2.log",
"/non/existant/file",
dir + "non_existent_file",
}

acc := testutil.Accumulator{}
Expand All @@ -37,7 +42,7 @@ func TestGatherNoMd5(t *testing.T) {
require.True(t, acc.HasPoint("filestat", tags2, "exists", int64(1)))

tags3 := map[string]string{
"file": "/non/existant/file",
"file": dir + "non_existent_file",
}
require.True(t, acc.HasPoint("filestat", tags3, "exists", int64(0)))
}
Expand All @@ -50,7 +55,7 @@ func TestGatherExplicitFiles(t *testing.T) {
fs.Files = []string{
dir + "log1.log",
dir + "log2.log",
"/non/existant/file",
dir + "non_existent_file",
}

acc := testutil.Accumulator{}
Expand All @@ -71,7 +76,7 @@ func TestGatherExplicitFiles(t *testing.T) {
require.True(t, acc.HasPoint("filestat", tags2, "md5_sum", "d41d8cd98f00b204e9800998ecf8427e"))

tags3 := map[string]string{
"file": "/non/existant/file",
"file": dir + "non_existent_file",
}
require.True(t, acc.HasPoint("filestat", tags3, "exists", int64(0)))
}
Expand Down Expand Up @@ -157,17 +162,18 @@ func TestModificationTime(t *testing.T) {
}

func TestNoModificationTime(t *testing.T) {
dir := getTestdataDir()
fs := NewFileStat()
fs.Log = testutil.Logger{}
fs.Files = []string{
"/non/existant/file",
dir + "non_existent_file",
}

acc := testutil.Accumulator{}
acc.GatherError(fs.Gather)

tags1 := map[string]string{
"file": "/non/existant/file",
"file": dir + "non_existent_file",
}
require.True(t, acc.HasPoint("filestat", tags1, "exists", int64(0)))
require.False(t, acc.HasInt64Field("filestat", "modification_time"))
Expand All @@ -185,5 +191,9 @@ func TestGetMd5(t *testing.T) {

func getTestdataDir() string {
_, filename, _, _ := runtime.Caller(1)
return strings.Replace(filename, "filestat_test.go", "testdata/", 1)
testdataDir := strings.Replace(filename, "filestat_test.go", "testdata/", 1)

// runtime.Caller always returns '/' as path separator https://github.com/golang/go/issues/3335
// make sure that path separator is always OS dependent
return strings.ReplaceAll(testdataDir, "/", string(os.PathSeparator))
}
11 changes: 7 additions & 4 deletions plugins/inputs/haproxy/haproxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"net"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"strings"
"testing"

Expand Down Expand Up @@ -114,12 +116,13 @@ func TestHaproxyGeneratesMetricsWithoutAuthentication(t *testing.T) {
func TestHaproxyGeneratesMetricsUsingSocket(t *testing.T) {
var randomNumber int64
var sockets [5]net.Listener
_globmask := "/tmp/test-haproxy*.sock"
_badmask := "/tmp/test-fail-haproxy*.sock"

_globmask := filepath.Join(os.TempDir(), "test-haproxy*.sock")
_badmask := filepath.Join(os.TempDir(), "test-fail-haproxy*.sock")

for i := 0; i < 5; i++ {
binary.Read(rand.Reader, binary.LittleEndian, &randomNumber)
sockname := fmt.Sprintf("/tmp/test-haproxy%d.sock", randomNumber)
sockname := filepath.Join(os.TempDir(), fmt.Sprintf("test-haproxy%d.sock", randomNumber))

sock, err := net.Listen("unix", sockname)
if err != nil {
Expand All @@ -146,7 +149,7 @@ func TestHaproxyGeneratesMetricsUsingSocket(t *testing.T) {

for _, sock := range sockets {
tags := map[string]string{
"server": sock.Addr().String(),
"server": getSocketAddr(sock.Addr().String()),
"proxy": "git",
"sv": "www",
"type": "server",
Expand Down
7 changes: 5 additions & 2 deletions plugins/inputs/http_response/http_response.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,11 @@ func localAddress(interfaceName string) (net.Addr, error) {

for _, addr := range addrs {
if naddr, ok := addr.(*net.IPNet); ok {
// leaving port set to zero to let kernel pick
return &net.TCPAddr{IP: naddr.IP}, nil
//need to choose IPv4 address
if len(naddr.Mask) == net.IPv4len {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this change. Why do we special case IPv4 here? Is this a generic fix or specific to windows? It looks like it will break IPv6 users.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are completely right. Here we have much deeper problem, I opened an issue for it: #8451. I will rollback this change in http_response.go. Till it is not fixed, I will disable http_response_test.go for Windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Good idea separating this out

// leaving port set to zero to let kernel pick
return &net.TCPAddr{IP: naddr.IP}, nil
}
}
}

Expand Down
Loading