Skip to content

Commit f7a3691

Browse files
filebeat: fix misnamed OS-suffixed test files so they actually run (#51853) (#51917)
Four test files in filebeat/input/ embedded a GOOS token in the middle of their name (`*_test_<goos>.go`) instead of ending in `_test.go`. Go only treats files ending in `_test.go` as test files, and it derives an implicit build constraint from a trailing `_<goos>` suffix. The combination meant these files were never considered tests on any platform: - filestream/{filestream,fswatch,identifier}_test_non_windows.go carried `//go:build !windows`, while the `_windows` suffix imposed an implicit `GOOS=windows` constraint. `windows && !windows` is never satisfiable, so they were dropped on every platform (they showed up in .IgnoredGoFiles). - file/identifier_test_windows.go (`//go:build windows`) did compile on Windows, but as ordinary package source rather than a test file, so `go test` never registered its Test func -- and it linked `testing` into the production Windows binary of the file input. As a result, 7 Test functions covering symlink handling, file rename/remove/truncate detection and inode-marker identity had not run since ~2020, hiding any regressions from CI. Because they never compiled, they had also drifted out of sync with the code under test. Changes: - Rename to valid, correctly-constrained test files, keeping the build tags: filestream_test_non_windows.go -> filestream_nonwindows_test.go identifier_test_non_windows.go -> identifier_nonwindows_test.go file/identifier_test_windows.go -> identifier_windows_test.go - Repair the reactivated filestream tests against current APIs (newFileReader/newFileScanner signatures, FSEvent.Descriptor instead of the removed FSEvent.Info, modern test helpers, t.TempDir). - Delete fswatch_test_non_windows.go: its three tests (TestFileScannerSymlinks, TestFileWatcherRenamedFile, TestFileWatcherRenamedTruncated) duplicate coverage already provided by the live TestFileScanner, TestFileWatcher and TestFileWatcherCopyTruncateWithFingerprint, and depended on the removed fileWatcher.sameFileFunc injection point. Net effect: TestLogFileRenamed, TestLogFileRemoved and TestFileIdentifierInodeMarker now run on non-Windows, TestInodeMarkerError now runs on Windows, and the file input's production Windows binary no longer imports testing. fswatch_test_non_windows.go was deletes, other tests already covered what it was supposed to cover. Assisted-By: Claude Code (cherry picked from commit 2a20583) Co-authored-by: Anderson Queiroz <anderson.queiroz@elastic.co>
1 parent e56aeac commit f7a3691

4 files changed

Lines changed: 31 additions & 256 deletions

File tree

File renamed without changes.

filebeat/input/filestream/filestream_test_non_windows.go renamed to filebeat/input/filestream/filestream_nonwindows_test.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,24 +22,26 @@ package filestream
2222
import (
2323
"context"
2424
"os"
25+
"path/filepath"
2526
"testing"
2627
"time"
2728

2829
"github.com/stretchr/testify/assert"
30+
"github.com/stretchr/testify/require"
2931

3032
"github.com/elastic/elastic-agent-libs/logp/logptest"
3133
)
3234

3335
// these tests are separated as one cannot delete/rename files
3436
// while another process is working with it on Windows
3537
func TestLogFileRenamed(t *testing.T) {
36-
f := createTestLogFile()
38+
f := createTestLogFile(t)
3739
defer f.Close()
3840

3941
renamedFile := f.Name() + ".renamed"
4042

4143
reader, _, err := newFileReader(
42-
logptest.NewTestingLogger(t, ""),
44+
logptest.NewFileLogger(t, filepath.Join("..", "..", "build", "integration-tests")).Logger,
4345
context.TODO(),
4446
f,
4547
readerConfig{},
@@ -57,7 +59,7 @@ func TestLogFileRenamed(t *testing.T) {
5759

5860
buf := make([]byte, 1024)
5961
_, err = reader.Read(buf)
60-
assert.Nil(t, err)
62+
assert.NoError(t, err)
6163

6264
err = os.Rename(f.Name(), renamedFile)
6365
if err != nil {
@@ -71,11 +73,11 @@ func TestLogFileRenamed(t *testing.T) {
7173
}
7274

7375
func TestLogFileRemoved(t *testing.T) {
74-
f := createTestLogFile()
76+
f := createTestLogFile(t)
7577
defer f.Close()
7678

7779
reader, _, err := newFileReader(
78-
logptest.NewTestingLogger(t, ""),
80+
logptest.NewFileLogger(t, filepath.Join("..", "..", "build", "integration-tests")).Logger,
7981
context.TODO(),
8082
f,
8183
readerConfig{},
@@ -93,7 +95,7 @@ func TestLogFileRemoved(t *testing.T) {
9395

9496
buf := make([]byte, 1024)
9597
_, err = reader.Read(buf)
96-
assert.Nil(t, err)
98+
assert.NoError(t, err)
9799

98100
err = os.Remove(f.Name())
99101
if err != nil {
@@ -104,3 +106,16 @@ func TestLogFileRemoved(t *testing.T) {
104106

105107
assert.Equal(t, ErrClosed, err)
106108
}
109+
110+
// createTestLogFile creates a temporary plain-text log file with a few lines of
111+
// content, wrapped as a filestream File ready to be passed to newFileReader.
112+
func createTestLogFile(t *testing.T) File {
113+
t.Helper()
114+
fs := filestream{
115+
readerConfig: readerConfig{BufferSize: 512},
116+
compression: CompressionNone,
117+
}
118+
f, err := fs.newFile(createTestPlainLogFile(t))
119+
require.NoError(t, err, "could not create test log file")
120+
return f
121+
}

filebeat/input/filestream/fswatch_test_non_windows.go

Lines changed: 0 additions & 240 deletions
This file was deleted.

filebeat/input/filestream/identifier_test_non_windows.go renamed to filebeat/input/filestream/identifier_nonwindows_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
package filestream
2121

2222
import (
23-
"io/ioutil"
2423
"os"
24+
"path/filepath"
2525
"testing"
2626

2727
"github.com/stretchr/testify/assert"
@@ -30,22 +30,23 @@ import (
3030
loginp "github.com/elastic/beats/v7/filebeat/input/filestream/internal/input-logfile"
3131
"github.com/elastic/beats/v7/libbeat/common/file"
3232
conf "github.com/elastic/elastic-agent-libs/config"
33+
"github.com/elastic/elastic-agent-libs/logp/logptest"
3334
)
3435

3536
func TestFileIdentifierInodeMarker(t *testing.T) {
3637
t.Run("inode_marker file identifier", func(t *testing.T) {
3738
const markerContents = "unique_marker"
38-
markerFile, err := ioutil.TempFile("", "test_file_identifier_inode_marker_identifier")
39+
dir := t.TempDir()
40+
markerFile, err := os.Create(filepath.Join(dir, "marker"))
3941
if err != nil {
4042
t.Fatalf("cannot create marker file for test: %v", err)
4143
}
42-
defer os.Remove(markerFile.Name())
4344

4445
_, err = markerFile.Write([]byte(markerContents))
4546
if err != nil {
4647
t.Fatalf("cannot write marker contents to file: %v", err)
4748
}
48-
markerFile.Sync()
49+
require.NoError(t, markerFile.Sync())
4950

5051
c := conf.MustNewConfigFrom(map[string]interface{}{
5152
"identifier": map[string]interface{}{
@@ -58,24 +59,23 @@ func TestFileIdentifierInodeMarker(t *testing.T) {
5859
err = c.Unpack(&cfg)
5960
require.NoError(t, err)
6061

61-
identifier, err := newFileIdentifier(cfg.Identifier)
62+
identifier, err := newFileIdentifier(cfg.Identifier, "", logptest.NewTestingLogger(t, ""))
6263
require.NoError(t, err)
6364
assert.Equal(t, inodeMarkerName, identifier.Name())
6465

65-
tmpFile, err := ioutil.TempFile("", "test_file_identifier_inode_marker")
66+
tmpFile, err := os.Create(filepath.Join(dir, "target"))
6667
if err != nil {
6768
t.Fatalf("cannot create temporary file for test: %v", err)
6869
}
69-
defer os.Remove(tmpFile.Name())
7070

7171
fi, err := tmpFile.Stat()
7272
if err != nil {
7373
t.Fatalf("cannot stat temporary file for test: %v", err)
7474
}
7575

7676
fsEvent := loginp.FSEvent{
77-
NewPath: tmpFile.Name(),
78-
Info: fi,
77+
NewPath: tmpFile.Name(),
78+
Descriptor: loginp.FileDescriptor{Info: file.ExtendFileInfo(fi)},
7979
}
8080
src := identifier.GetSource(fsEvent)
8181

@@ -87,7 +87,7 @@ func TestFileIdentifierInodeMarker(t *testing.T) {
8787
if err != nil {
8888
t.Fatalf("cannot write marker contents to file: %v", err)
8989
}
90-
markerFile.Sync()
90+
require.NoError(t, markerFile.Sync())
9191

9292
src = identifier.GetSource(fsEvent)
9393

0 commit comments

Comments
 (0)