From 02e6e1da1a358da963b726b8bc52bdc733fb709d Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 15:22:09 -0700 Subject: [PATCH 01/28] First pass at running on windows action --- .github/workflows/test.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c0290b65..7bedaec2 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -20,11 +20,12 @@ jobs: - uses: actions/checkout@v3 - uses: Jerome1337/gofmt-action@v1.0.5 tests: - runs-on: ubuntu-latest strategy: matrix: goversion: ['1.21', '1.22'] - name: Go ${{ matrix.goversion }} tests + os: ['ubuntu-latest', 'windows-latest'] + name: Go ${{ matrix.goversion }} (${{ matrix.os}}) tests + runs-on: ${{ matrix.os }} env: SW_APM_DEBUG_LEVEL: 1 steps: From 3447c2b9c2cd12729014b21e3da352b4e34b5037 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 15:32:56 -0700 Subject: [PATCH 02/28] Try using an envvar --- internal/reporter/certgen.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/reporter/certgen.sh b/internal/reporter/certgen.sh index c6b8409a..5c4217d7 100755 --- a/internal/reporter/certgen.sh +++ b/internal/reporter/certgen.sh @@ -14,11 +14,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +CONFIG< Date: Tue, 30 Jul 2024 15:39:22 -0700 Subject: [PATCH 03/28] Write a config file --- internal/reporter/certgen.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/reporter/certgen.sh b/internal/reporter/certgen.sh index 5c4217d7..c62049f9 100755 --- a/internal/reporter/certgen.sh +++ b/internal/reporter/certgen.sh @@ -14,12 +14,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -CONFIG< $CONFIG openssl req -x509 -newkey rsa:4096 -sha256 -days 365 -nodes \ -keyout for_test.key -out for_test.crt -extensions san -config "$CONFIG" \ -subj "/CN=localhost" +rm $CONFIG \ No newline at end of file From 1be17a0a282aacbd53f23c0be3bae0faf1e2c582 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 16:08:34 -0700 Subject: [PATCH 04/28] platform indepedendent errs, one windows-specific string check --- internal/config/config_test.go | 9 ++++++++- internal/host/k8s/k8s_test.go | 7 +++---- internal/uams/file_test.go | 4 +++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e8b0f7cf..8dd197d0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -18,6 +18,7 @@ import ( "io" "os" "path/filepath" + "runtime" "strings" "testing" @@ -474,7 +475,13 @@ func TestInvalidConfigFile(t *testing.T) { os.Setenv("SW_APM_SERVICE_KEY", "ae38315f6116585d64d82ec2455aa3ec61e02fee25d286f74ace9e4fea189217:go") os.Setenv("SW_APM_CONFIG_FILE", "/tmp/file-not-exist.yaml") _ = NewConfig() - assert.Contains(t, buf.String(), "no such file or directory") + var exp string + if runtime.GOOS == "windows" { + exp = "The system cannot find the path specified." + } else { + exp = "no such file or directory" + } + assert.Contains(t, buf.String(), exp) } func TestInvalidConfig(t *testing.T) { diff --git a/internal/host/k8s/k8s_test.go b/internal/host/k8s/k8s_test.go index 20cd531f..da3db919 100644 --- a/internal/host/k8s/k8s_test.go +++ b/internal/host/k8s/k8s_test.go @@ -96,7 +96,7 @@ func TestRequestMetadataFromEnv(t *testing.T) { md, err := requestMetadata() require.Error(t, err) require.Nil(t, md) - require.Equal(t, "open /run/secrets/kubernetes.io/serviceaccount/namespace: no such file or directory", err.Error()) + require.True(t, os.IsNotExist(err)) defer testutils.Setenv(t, "SW_K8S_POD_NAMESPACE", "my env namespace")() md, err = requestMetadata() @@ -124,7 +124,7 @@ func TestRequestMetadataNoNamespace(t *testing.T) { md, err := requestMetadata() require.Error(t, err) require.Nil(t, md) - require.Equal(t, fmt.Sprintf("open %s: no such file or directory", determineNamspaceFileForOS()), err.Error()) + require.True(t, os.IsNotExist(err)) } func TestMetadata_ToPB(t *testing.T) { @@ -159,9 +159,8 @@ func TestGetNamespaceNoneFound(t *testing.T) { require.NoError(t, os.Unsetenv("SW_K8S_POD_NAMESPACE")) ns, err := getNamespaceFromFile("this file does not exist and should not be opened") require.Error(t, err) - require.Equal(t, "open this file does not exist and should not be opened: no such file or directory", err.Error()) + require.True(t, os.IsNotExist(err)) require.Equal(t, "", ns) - } // Test getPodName diff --git a/internal/uams/file_test.go b/internal/uams/file_test.go index 0c4d9fa5..fe2d8f83 100644 --- a/internal/uams/file_test.go +++ b/internal/uams/file_test.go @@ -15,9 +15,11 @@ package uams import ( + "errors" "fmt" "github.com/google/uuid" "github.com/stretchr/testify/require" + "io/fs" "os" "testing" ) @@ -29,7 +31,7 @@ func TestReadFromFileNoExists(t *testing.T) { uid, err := ReadFromFile(testfile) require.Equal(t, uuid.Nil, uid) require.Error(t, err) - require.Equal(t, "could not stat uams client file: stat /tmp/foobarbaz: no such file or directory", err.Error()) + require.True(t, errors.Is(err, fs.ErrNotExist)) } func TestReadFromFileDirectory(t *testing.T) { From 91668b1ae4f1f69986fd1858dcbcfd0539442f02 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 16:14:14 -0700 Subject: [PATCH 05/28] Use require --- internal/config/config_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 8dd197d0..aa3b831d 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -343,10 +343,10 @@ func TestYamlConfig(t *testing.T) { } out, err := yaml.Marshal(&yamlConfig) - assert.Nil(t, err) + require.NoError(t, err) err = os.WriteFile("/tmp/solarwinds-apm-config.yaml", out, 0644) - assert.Nil(t, err) + require.NoError(t, err) // Test with config file ClearEnvs() From e5de4ad015826e0c368e44f53dd85d0bb78068db Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 16:32:24 -0700 Subject: [PATCH 06/28] Add missing file close --- internal/host/k8s/k8s.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/host/k8s/k8s.go b/internal/host/k8s/k8s.go index 49978386..952aa520 100644 --- a/internal/host/k8s/k8s.go +++ b/internal/host/k8s/k8s.go @@ -169,6 +169,7 @@ func getPodUidFromFile(fn string) (string, error) { func getPodUidFromProc(fn string) (string, error) { f, err := os.Open(fn) + defer f.Close() if err != nil { return "", err } From fae665705694993ef47d15de83debb7f96f7bd3f Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 16:39:24 -0700 Subject: [PATCH 07/28] Handle error properly on file close --- internal/host/k8s/k8s.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/host/k8s/k8s.go b/internal/host/k8s/k8s.go index 952aa520..f92c2ff8 100644 --- a/internal/host/k8s/k8s.go +++ b/internal/host/k8s/k8s.go @@ -169,10 +169,14 @@ func getPodUidFromFile(fn string) (string, error) { func getPodUidFromProc(fn string) (string, error) { f, err := os.Open(fn) - defer f.Close() if err != nil { return "", err } + defer func() { + if err := f.Close(); err != nil { + log.Warningf("Could not close file: %s", err) + } + }() scanner := bufio.NewScanner(f) for scanner.Scan() { line := scanner.Text() From 6184ac84da57c8cd40e134268ee29151083d353f Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 17:10:34 -0700 Subject: [PATCH 08/28] Handle connrefused err --- internal/uams/http_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/uams/http_test.go b/internal/uams/http_test.go index e74c2dcf..20a16ccd 100644 --- a/internal/uams/http_test.go +++ b/internal/uams/http_test.go @@ -15,11 +15,13 @@ package uams import ( + "errors" "fmt" "github.com/google/uuid" "github.com/solarwinds/apm-go/internal/testutils" "github.com/stretchr/testify/require" "net/http" + "syscall" "testing" ) @@ -27,7 +29,7 @@ func TestReadFromHttpConnRefused(t *testing.T) { uid, err := ReadFromHttp("http://127.0.0.1:12345") require.Error(t, err) require.Equal(t, uuid.Nil, uid) - require.Equal(t, `Get "http://127.0.0.1:12345": dial tcp 127.0.0.1:12345: connect: connection refused`, err.Error()) + require.True(t, errors.Is(err, syscall.ECONNREFUSED)) } func TestReadFromHttp(t *testing.T) { From 5f8b22d4fcd5bff1b47bbcf3886418e4001f3d11 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Tue, 30 Jul 2024 17:31:17 -0700 Subject: [PATCH 09/28] Updates to certgen.sh --- internal/reporter/certgen.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/reporter/certgen.sh b/internal/reporter/certgen.sh index c62049f9..2c6bff18 100755 --- a/internal/reporter/certgen.sh +++ b/internal/reporter/certgen.sh @@ -16,11 +16,14 @@ CONFIG=$(mktemp) echo "[req] -distinguished_name=req +distinguished_name=req_distinguished_name +prompt=no [san] subjectAltName=DNS:localhost +[req_distinguished_name] +CN=localhost " > $CONFIG +echo $CONFIG openssl req -x509 -newkey rsa:4096 -sha256 -days 365 -nodes \ - -keyout for_test.key -out for_test.crt -extensions san -config "$CONFIG" \ - -subj "/CN=localhost" + -keyout for_test.key -out for_test.crt -extensions san -config "$CONFIG" rm $CONFIG \ No newline at end of file From f16177feecd3e433c0a3a9c25c1868420ac8c92b Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 09:40:00 -0700 Subject: [PATCH 10/28] Misc test updates --- internal/host/azure/azure_test.go | 10 ++++++---- internal/oboe/settings_lambda_test.go | 2 ++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/host/azure/azure_test.go b/internal/host/azure/azure_test.go index 28f95674..aff0a7d4 100644 --- a/internal/host/azure/azure_test.go +++ b/internal/host/azure/azure_test.go @@ -15,10 +15,12 @@ package azure import ( + "errors" "github.com/solarwinds/apm-go/internal/testutils" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "net/http" + "syscall" "testing" ) @@ -40,10 +42,10 @@ func TestQueryAzureInvalidJson(t *testing.T) { func TestQueryAzureNoHttpResponse(t *testing.T) { m, err := queryAzureIMDS("http://127.0.0.1:12345/asdf") - assert.Error(t, err) - assert.Nil(t, m) - assert.Equal(t, `Get "http://127.0.0.1:12345/asdf?api-version=2021-12-13&format=json": dial tcp 127.0.0.1:12345: connect: connection refused`, err.Error()) - assert.Nil(t, m.ToPB()) + require.Error(t, err) + require.Nil(t, m) + require.True(t, errors.Is(err, syscall.ECONNREFUSED)) + require.Nil(t, m.ToPB()) } func TestQueryAzureIMDS(t *testing.T) { diff --git a/internal/oboe/settings_lambda_test.go b/internal/oboe/settings_lambda_test.go index a495c371..a471e388 100644 --- a/internal/oboe/settings_lambda_test.go +++ b/internal/oboe/settings_lambda_test.go @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +//go:build !windows + package oboe import ( From 5043b6a787a7868be33cb3061b45d65f7cb4c089 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 11:18:00 -0700 Subject: [PATCH 11/28] Misc test updates --- internal/config/config_test.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index aa3b831d..794e1020 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -345,12 +345,18 @@ func TestYamlConfig(t *testing.T) { out, err := yaml.Marshal(&yamlConfig) require.NoError(t, err) - err = os.WriteFile("/tmp/solarwinds-apm-config.yaml", out, 0644) + f, err := os.CreateTemp("", "*-test-config.yaml") + defer func() { + _ = f.Close() + os.Remove(f.Name()) + }() + require.NoError(t, err) + err = os.WriteFile(f.Name(), out, 0644) require.NoError(t, err) // Test with config file ClearEnvs() - os.Setenv(envSolarWindsAPMConfigFile, "/tmp/solarwinds-apm-config.yaml") + os.Setenv(envSolarWindsAPMConfigFile, f.Name()) c := NewConfig() assert.Equal(t, &yamlConfig, c) @@ -378,7 +384,7 @@ func TestYamlConfig(t *testing.T) { } ClearEnvs() SetEnvs(envs) - os.Setenv("SW_APM_CONFIG_FILE", "/tmp/solarwinds-apm-config.yaml") + os.Setenv("SW_APM_CONFIG_FILE", f.Name()) envConfig := Config{ Collector: "collector.test.com", @@ -459,11 +465,17 @@ func TestInvalidConfigFile(t *testing.T) { log.SetOutput(os.Stderr) log.SetLevel(oldLevel) }() + f, err := os.CreateTemp("", "*-test-config.json") + require.NoError(t, err) + defer func() { + _ = f.Close() + os.Remove(f.Name()) + }() ClearEnvs() os.Setenv("SW_APM_SERVICE_KEY", "ae38315f6116585d64d82ec2455aa3ec61e02fee25d286f74ace9e4fea189217:go") - os.Setenv("SW_APM_CONFIG_FILE", "/tmp/solarwinds-apm-config.json") - require.NoError(t, os.WriteFile("/tmp/solarwinds-apm-config.json", []byte("hello"), 0644)) + os.Setenv("SW_APM_CONFIG_FILE", f.Name()) + require.NoError(t, os.WriteFile(f.Name(), []byte("hello"), 0644)) _ = NewConfig() assert.Contains(t, buf.String(), ErrUnsupportedFormat.Error()) From 23e49f882a38bc572f69ebc2a106f1e4a15e8075 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 14:00:03 -0700 Subject: [PATCH 12/28] Test if setting the path to C: helps --- .github/workflows/test.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 7bedaec2..82f9c21e 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -24,6 +24,11 @@ jobs: matrix: goversion: ['1.21', '1.22'] os: ['ubuntu-latest', 'windows-latest'] + include: + - os: windows-latest + path: 'C:/repo' + - os: ubuntu-latest + path: '/repo' name: Go ${{ matrix.goversion }} (${{ matrix.os}}) tests runs-on: ${{ matrix.os }} env: From 036d44f3ea0f214d2fa265db434fe51017443dd7 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 14:07:16 -0700 Subject: [PATCH 13/28] Add forgotten matrix path --- .github/workflows/test.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 82f9c21e..8c09dc5b 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -35,6 +35,8 @@ jobs: SW_APM_DEBUG_LEVEL: 1 steps: - uses: actions/checkout@v3 + with: + path: ${{ matrix.path }} - uses: actions/setup-go@v4 with: go-version: ${{ matrix.goversion }} From c0a58b3f4c36f7b40b82653e54bf503fcfff06ab Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 14:11:41 -0700 Subject: [PATCH 14/28] Restore previous build yaml --- .github/workflows/test.yaml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 8c09dc5b..7bedaec2 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -24,19 +24,12 @@ jobs: matrix: goversion: ['1.21', '1.22'] os: ['ubuntu-latest', 'windows-latest'] - include: - - os: windows-latest - path: 'C:/repo' - - os: ubuntu-latest - path: '/repo' name: Go ${{ matrix.goversion }} (${{ matrix.os}}) tests runs-on: ${{ matrix.os }} env: SW_APM_DEBUG_LEVEL: 1 steps: - uses: actions/checkout@v3 - with: - path: ${{ matrix.path }} - uses: actions/setup-go@v4 with: go-version: ${{ matrix.goversion }} From d13a9fd15dc8405b2f2fdc25df7b598bd153ff71 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 14:15:48 -0700 Subject: [PATCH 15/28] Skip reporter_grpc_test.go on windows --- internal/reporter/reporter_grpc_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/reporter/reporter_grpc_test.go b/internal/reporter/reporter_grpc_test.go index 198d6865..b1fc68de 100644 --- a/internal/reporter/reporter_grpc_test.go +++ b/internal/reporter/reporter_grpc_test.go @@ -12,6 +12,19 @@ // See the License for the specific language governing permissions and // limitations under the License. +// There's an issue in GitHub Actions running Windows that causes `net.Listen` +// to fail with: +// +// listen tcp: lookup localhost: getaddrinfow: A non-recoverable error +// occurred during a database lookup. +// +// For now, we're going to rely on non-windows to verify that this code works. +// Over time, the (non-OTLP) grpc implementation will be phased out, so I think +// this is a reasonable compromise. +// -- @swi-jared + +//go:build !windows + package reporter import ( From 0321a0130ebeb2038f5015a55dd6b1255c61ecb5 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 14:20:01 -0700 Subject: [PATCH 16/28] Skip reporter_test.go on windows --- internal/reporter/reporter_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/reporter/reporter_test.go b/internal/reporter/reporter_test.go index f3c1548d..eeed2a2d 100644 --- a/internal/reporter/reporter_test.go +++ b/internal/reporter/reporter_test.go @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +// See note in `reporter_grpc_test.go` +//go:build !windows + package reporter import ( From 4b26227251adf4dfcc12e5adfd727fa178ccaabe Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 14:31:24 -0700 Subject: [PATCH 17/28] Add some test error debug --- internal/host/azure/azure_test.go | 2 +- internal/uams/http_test.go | 2 +- internal/uams/uams_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/host/azure/azure_test.go b/internal/host/azure/azure_test.go index aff0a7d4..0486830a 100644 --- a/internal/host/azure/azure_test.go +++ b/internal/host/azure/azure_test.go @@ -44,7 +44,7 @@ func TestQueryAzureNoHttpResponse(t *testing.T) { m, err := queryAzureIMDS("http://127.0.0.1:12345/asdf") require.Error(t, err) require.Nil(t, m) - require.True(t, errors.Is(err, syscall.ECONNREFUSED)) + require.True(t, errors.Is(err, syscall.ECONNREFUSED), "%+v", err) require.Nil(t, m.ToPB()) } diff --git a/internal/uams/http_test.go b/internal/uams/http_test.go index 20a16ccd..bb49dab0 100644 --- a/internal/uams/http_test.go +++ b/internal/uams/http_test.go @@ -29,7 +29,7 @@ func TestReadFromHttpConnRefused(t *testing.T) { uid, err := ReadFromHttp("http://127.0.0.1:12345") require.Error(t, err) require.Equal(t, uuid.Nil, uid) - require.True(t, errors.Is(err, syscall.ECONNREFUSED)) + require.True(t, errors.Is(err, syscall.ECONNREFUSED), "%+v", err) } func TestReadFromHttp(t *testing.T) { diff --git a/internal/uams/uams_test.go b/internal/uams/uams_test.go index 353d6e56..890a4bb7 100644 --- a/internal/uams/uams_test.go +++ b/internal/uams/uams_test.go @@ -43,7 +43,7 @@ func TestUpdateClientId(t *testing.T) { require.Equal(t, uid, currState.clientId) require.Equal(t, "file", currState.via) require.True(t, currState.updated.After(a)) - require.True(t, currState.updated.Before(b)) + require.True(t, currState.updated.Before(b), "currState.updated: %s, is not before %s", currState.updated.String(), b.String()) require.Equal(t, uid, GetCurrentClientId()) } From 1e17c9c5e9b37df6a96e422e345c0c832af7e99e Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 14:41:28 -0700 Subject: [PATCH 18/28] Wrong test --- internal/uams/uams_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/uams/uams_test.go b/internal/uams/uams_test.go index 890a4bb7..2cfa41a9 100644 --- a/internal/uams/uams_test.go +++ b/internal/uams/uams_test.go @@ -42,7 +42,7 @@ func TestUpdateClientId(t *testing.T) { b := time.Now() require.Equal(t, uid, currState.clientId) require.Equal(t, "file", currState.via) - require.True(t, currState.updated.After(a)) + require.True(t, currState.updated.After(a), "currState.updated: %s, is not after %s", currState.updated.String(), a.String()) require.True(t, currState.updated.Before(b), "currState.updated: %s, is not before %s", currState.updated.String(), b.String()) require.Equal(t, uid, GetCurrentClientId()) From b79e4d0f7ce670f43876cd1d0526c79fd5e7c869 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 15:10:20 -0700 Subject: [PATCH 19/28] Add a touch of delay for Windows --- internal/uams/uams_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/internal/uams/uams_test.go b/internal/uams/uams_test.go index 2cfa41a9..36818fd3 100644 --- a/internal/uams/uams_test.go +++ b/internal/uams/uams_test.go @@ -22,6 +22,7 @@ import ( "net/http" "os" "path/filepath" + "runtime" "testing" "time" ) @@ -38,7 +39,15 @@ func TestUpdateClientId(t *testing.T) { uid, err := uuid.NewRandom() require.NoError(t, err) a := time.Now() + // windows doesn't seem to notice that a few nanoseconds have passed, so we + // introduce a touch of delay + if runtime.GOOS == "windows" { + time.Sleep(time.Millisecond) + } updateClientId(uid, "file") + if runtime.GOOS == "windows" { + time.Sleep(time.Millisecond) + } b := time.Now() require.Equal(t, uid, currState.clientId) require.Equal(t, "file", currState.via) From f7d126e18a53ef96e576648c7c11e3aa9d5f6031 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 15:28:00 -0700 Subject: [PATCH 20/28] Misc windows-related fixes --- internal/host/azure/azure_test.go | 3 +-- internal/testutils/testutils_unix.go | 21 +++++++++++++++++++++ internal/testutils/testutils_windows.go | 21 +++++++++++++++++++++ internal/uams/http_test.go | 3 +-- 4 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 internal/testutils/testutils_unix.go create mode 100644 internal/testutils/testutils_windows.go diff --git a/internal/host/azure/azure_test.go b/internal/host/azure/azure_test.go index 0486830a..05b239b1 100644 --- a/internal/host/azure/azure_test.go +++ b/internal/host/azure/azure_test.go @@ -20,7 +20,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "net/http" - "syscall" "testing" ) @@ -44,7 +43,7 @@ func TestQueryAzureNoHttpResponse(t *testing.T) { m, err := queryAzureIMDS("http://127.0.0.1:12345/asdf") require.Error(t, err) require.Nil(t, m) - require.True(t, errors.Is(err, syscall.ECONNREFUSED), "%+v", err) + require.True(t, errors.Is(err, testutils.ConnectionRefusedError), "%+v", err) require.Nil(t, m.ToPB()) } diff --git a/internal/testutils/testutils_unix.go b/internal/testutils/testutils_unix.go new file mode 100644 index 00000000..8b95de0b --- /dev/null +++ b/internal/testutils/testutils_unix.go @@ -0,0 +1,21 @@ +// © 2023 SolarWinds Worldwide, LLC. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !windows + +package testutils + +import "syscall" + +const ConnectionRefusedError = syscall.ECONNREFUSED diff --git a/internal/testutils/testutils_windows.go b/internal/testutils/testutils_windows.go new file mode 100644 index 00000000..ea48cbea --- /dev/null +++ b/internal/testutils/testutils_windows.go @@ -0,0 +1,21 @@ +// © 2023 SolarWinds Worldwide, LLC. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package testutils + +import "syscall" + +const ConnectionRefusedError = syscall.WSAECONNREFUSED diff --git a/internal/uams/http_test.go b/internal/uams/http_test.go index bb49dab0..54142a71 100644 --- a/internal/uams/http_test.go +++ b/internal/uams/http_test.go @@ -21,7 +21,6 @@ import ( "github.com/solarwinds/apm-go/internal/testutils" "github.com/stretchr/testify/require" "net/http" - "syscall" "testing" ) @@ -29,7 +28,7 @@ func TestReadFromHttpConnRefused(t *testing.T) { uid, err := ReadFromHttp("http://127.0.0.1:12345") require.Error(t, err) require.Equal(t, uuid.Nil, uid) - require.True(t, errors.Is(err, syscall.ECONNREFUSED), "%+v", err) + require.True(t, errors.Is(err, testutils.ConnectionRefusedError), "%+v", err) } func TestReadFromHttp(t *testing.T) { From cd360f9d4faa444e2ac1268608bc3df7687f176d Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 15:38:20 -0700 Subject: [PATCH 21/28] Create our own connrefused for Windows --- internal/host/azure/azure_test.go | 3 ++- internal/testutils/testutils_windows.go | 4 +--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/host/azure/azure_test.go b/internal/host/azure/azure_test.go index 05b239b1..0486830a 100644 --- a/internal/host/azure/azure_test.go +++ b/internal/host/azure/azure_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "net/http" + "syscall" "testing" ) @@ -43,7 +44,7 @@ func TestQueryAzureNoHttpResponse(t *testing.T) { m, err := queryAzureIMDS("http://127.0.0.1:12345/asdf") require.Error(t, err) require.Nil(t, m) - require.True(t, errors.Is(err, testutils.ConnectionRefusedError), "%+v", err) + require.True(t, errors.Is(err, syscall.ECONNREFUSED), "%+v", err) require.Nil(t, m.ToPB()) } diff --git a/internal/testutils/testutils_windows.go b/internal/testutils/testutils_windows.go index ea48cbea..63185ba1 100644 --- a/internal/testutils/testutils_windows.go +++ b/internal/testutils/testutils_windows.go @@ -16,6 +16,4 @@ package testutils -import "syscall" - -const ConnectionRefusedError = syscall.WSAECONNREFUSED +const ConnectionRefusedError = syscall.Errno(10061) From bb203c2344e3cad888784f960ffff54a56e1f762 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 15:38:45 -0700 Subject: [PATCH 22/28] oops --- internal/host/azure/azure_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/host/azure/azure_test.go b/internal/host/azure/azure_test.go index 0486830a..05b239b1 100644 --- a/internal/host/azure/azure_test.go +++ b/internal/host/azure/azure_test.go @@ -20,7 +20,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "net/http" - "syscall" "testing" ) @@ -44,7 +43,7 @@ func TestQueryAzureNoHttpResponse(t *testing.T) { m, err := queryAzureIMDS("http://127.0.0.1:12345/asdf") require.Error(t, err) require.Nil(t, m) - require.True(t, errors.Is(err, syscall.ECONNREFUSED), "%+v", err) + require.True(t, errors.Is(err, testutils.ConnectionRefusedError), "%+v", err) require.Nil(t, m.ToPB()) } From a34f8a090d8d154507dcffb4b9038611e3d15c0c Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 15:41:00 -0700 Subject: [PATCH 23/28] oops again --- internal/testutils/testutils_windows.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/testutils/testutils_windows.go b/internal/testutils/testutils_windows.go index 63185ba1..72497e75 100644 --- a/internal/testutils/testutils_windows.go +++ b/internal/testutils/testutils_windows.go @@ -16,4 +16,6 @@ package testutils +import "syscall" + const ConnectionRefusedError = syscall.Errno(10061) From 0759cec22fda1305cc7dd708b232202108bf3cad Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Wed, 31 Jul 2024 15:59:13 -0700 Subject: [PATCH 24/28] Let's see if this works --- internal/testutils/testutils_windows.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testutils/testutils_windows.go b/internal/testutils/testutils_windows.go index 72497e75..5f8fd6c5 100644 --- a/internal/testutils/testutils_windows.go +++ b/internal/testutils/testutils_windows.go @@ -16,6 +16,6 @@ package testutils -import "syscall" +import "golang.org/x/sys/windows" -const ConnectionRefusedError = syscall.Errno(10061) +const ConnectionRefusedError = windows.WSAECONNREFUSED From e329b339be231da4bbadc0f8e44ee4a195536533 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Thu, 1 Aug 2024 08:33:57 -0700 Subject: [PATCH 25/28] Try using a higher port for windows --- internal/reporter/reporter_grpc_test.go | 2 +- internal/reporter/reporter_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/reporter/reporter_grpc_test.go b/internal/reporter/reporter_grpc_test.go index b1fc68de..8023b03f 100644 --- a/internal/reporter/reporter_grpc_test.go +++ b/internal/reporter/reporter_grpc_test.go @@ -23,7 +23,7 @@ // this is a reasonable compromise. // -- @swi-jared -//go:build !windows +///go:build !windows package reporter diff --git a/internal/reporter/reporter_test.go b/internal/reporter/reporter_test.go index eeed2a2d..4e4944b9 100644 --- a/internal/reporter/reporter_test.go +++ b/internal/reporter/reporter_test.go @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -// See note in `reporter_grpc_test.go` -//go:build !windows +//See note in `reporter_grpc_test.go` +///go:build !windows package reporter @@ -84,7 +84,7 @@ func TestGRPCReporter(t *testing.T) { // start test gRPC server setEnv("SW_APM_DEBUG_LEVEL", "debug") config.Load() - addr := "localhost:4567" + addr := "localhost:10023" server := StartTestGRPCServer(t, addr) time.Sleep(100 * time.Millisecond) @@ -171,7 +171,7 @@ func TestGRPCReporter(t *testing.T) { func TestShutdownGRPCReporter(t *testing.T) { // start test gRPC server setEnv("SW_APM_DEBUG_LEVEL", "debug") - addr := "localhost:4567" + addr := "localhost:10023" server := StartTestGRPCServer(t, addr) time.Sleep(100 * time.Millisecond) @@ -227,7 +227,7 @@ func TestInvalidKey(t *testing.T) { setEnv("SW_APM_DEBUG_LEVEL", "debug") oldKey := os.Getenv("SW_APM_SERVICE_KEY") setEnv("SW_APM_SERVICE_KEY", invalidKey) - addr := "localhost:4567" + addr := "localhost:10023" setEnv("SW_APM_COLLECTOR", addr) setEnv("SW_APM_TRUSTEDPATH", testCertFile) @@ -475,7 +475,7 @@ func TestCollectMetricsNextInterval(t *testing.T) { // testProxy performs tests of http/https proxy. func testProxy(t *testing.T, proxyUrl string) { - addr := "localhost:4567" + addr := "localhost:10023" setEnv("SW_APM_DEBUG_LEVEL", "debug") setEnv("SW_APM_COLLECTOR", addr) From 1ec390899c76434d1b28b03f4999d657c8f57681 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Thu, 1 Aug 2024 08:37:17 -0700 Subject: [PATCH 26/28] Revert "Try using a higher port for windows" This reverts commit e329b339be231da4bbadc0f8e44ee4a195536533. --- internal/reporter/reporter_grpc_test.go | 2 +- internal/reporter/reporter_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/reporter/reporter_grpc_test.go b/internal/reporter/reporter_grpc_test.go index 8023b03f..b1fc68de 100644 --- a/internal/reporter/reporter_grpc_test.go +++ b/internal/reporter/reporter_grpc_test.go @@ -23,7 +23,7 @@ // this is a reasonable compromise. // -- @swi-jared -///go:build !windows +//go:build !windows package reporter diff --git a/internal/reporter/reporter_test.go b/internal/reporter/reporter_test.go index 4e4944b9..eeed2a2d 100644 --- a/internal/reporter/reporter_test.go +++ b/internal/reporter/reporter_test.go @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -//See note in `reporter_grpc_test.go` -///go:build !windows +// See note in `reporter_grpc_test.go` +//go:build !windows package reporter @@ -84,7 +84,7 @@ func TestGRPCReporter(t *testing.T) { // start test gRPC server setEnv("SW_APM_DEBUG_LEVEL", "debug") config.Load() - addr := "localhost:10023" + addr := "localhost:4567" server := StartTestGRPCServer(t, addr) time.Sleep(100 * time.Millisecond) @@ -171,7 +171,7 @@ func TestGRPCReporter(t *testing.T) { func TestShutdownGRPCReporter(t *testing.T) { // start test gRPC server setEnv("SW_APM_DEBUG_LEVEL", "debug") - addr := "localhost:10023" + addr := "localhost:4567" server := StartTestGRPCServer(t, addr) time.Sleep(100 * time.Millisecond) @@ -227,7 +227,7 @@ func TestInvalidKey(t *testing.T) { setEnv("SW_APM_DEBUG_LEVEL", "debug") oldKey := os.Getenv("SW_APM_SERVICE_KEY") setEnv("SW_APM_SERVICE_KEY", invalidKey) - addr := "localhost:10023" + addr := "localhost:4567" setEnv("SW_APM_COLLECTOR", addr) setEnv("SW_APM_TRUSTEDPATH", testCertFile) @@ -475,7 +475,7 @@ func TestCollectMetricsNextInterval(t *testing.T) { // testProxy performs tests of http/https proxy. func testProxy(t *testing.T, proxyUrl string) { - addr := "localhost:10023" + addr := "localhost:4567" setEnv("SW_APM_DEBUG_LEVEL", "debug") setEnv("SW_APM_COLLECTOR", addr) From 0d4b73c94b45e0e362c08358509cd1ef70692310 Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Thu, 1 Aug 2024 08:44:01 -0700 Subject: [PATCH 27/28] Change placement of err check --- internal/config/config_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 794e1020..3347ba4b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -346,11 +346,11 @@ func TestYamlConfig(t *testing.T) { require.NoError(t, err) f, err := os.CreateTemp("", "*-test-config.yaml") + require.NoError(t, err) defer func() { _ = f.Close() os.Remove(f.Name()) }() - require.NoError(t, err) err = os.WriteFile(f.Name(), out, 0644) require.NoError(t, err) From b7b08b1864a346d34809b67df45029e0f322abfd Mon Sep 17 00:00:00 2001 From: Jared Harper Date: Thu, 1 Aug 2024 08:50:20 -0700 Subject: [PATCH 28/28] Minor test cleanup --- internal/uams/uams_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/uams/uams_test.go b/internal/uams/uams_test.go index 36818fd3..063b8b11 100644 --- a/internal/uams/uams_test.go +++ b/internal/uams/uams_test.go @@ -51,8 +51,8 @@ func TestUpdateClientId(t *testing.T) { b := time.Now() require.Equal(t, uid, currState.clientId) require.Equal(t, "file", currState.via) - require.True(t, currState.updated.After(a), "currState.updated: %s, is not after %s", currState.updated.String(), a.String()) - require.True(t, currState.updated.Before(b), "currState.updated: %s, is not before %s", currState.updated.String(), b.String()) + require.True(t, currState.updated.After(a)) + require.True(t, currState.updated.Before(b)) require.Equal(t, uid, GetCurrentClientId()) }