[9.3](backport #51853) filebeat: fix misnamed OS-suffixed test files so they actually run#51919
[9.3](backport #51853) filebeat: fix misnamed OS-suffixed test files so they actually run#51919mergify[bot] wants to merge 2 commits into
Conversation
|
Cherry-pick of 2a20583 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
🤖 GitHub commentsJust comment with:
|
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
This comment has been minimized.
This comment has been minimized.
TL;DRAll four failed jobs have the same root cause: the backport left unresolved Git conflict markers in Remediation
Investigation detailsRoot CauseThe pull request is a Mergify backport whose cherry-pick reported conflicts in Evidence
VerificationNo additional tests were run because the checked-out workspace is not the pull request branch; the CI logs and PR diff provide direct, reproducible evidence of the failure. What is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
andrzej-stencel
left a comment
There was a problem hiding this comment.
Needs fixing conflicts
…51853) 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) # Conflicts: # filebeat/input/filestream/fswatch_test_non_windows.go # filebeat/input/filestream/identifier_nonwindows_test.go
846bbff to
f606532
Compare
Proposed commit message
Checklist
golangci-lint: 0 issues)I have made corresponding changes to the documentation(test-only)I have made corresponding change to the default configuration files(test-only)stresstest.shscript to run them under stress conditions and race detector to verify their stability. — the reactivated tests pass under-raceI have added an entry in(test-only change; apply the./changelog/fragmentsskip-changeloglabel)Disruptive User Impact
None. Test-only change. The one shipped-artifact effect is positive: the
fileinput's production Windows binary no longer links the
testingpackage.How to test this PR locally
1. The reactivated tests run and pass (this branch, under
-race)2. The filestream tests are no longer silently skipped
Run this in
filebeat/input/filestreamonmain, then again on this branch —it's the same command on both (the
non_?windowsregex matches both the old*_test_non_windows.goand the renamed*_nonwindows_test.gofiles):Before (
main) — compiled on no platform; ignored everywhere:After (this branch) — compiled as tests on non-Windows, ignored on Windows
(
fswatch_*is intentionally gone — deleted as redundant):3.
testingis no longer linked into the production Windowsfilebinarygo vetis not a useful check here — the misnamed file compiles as ordinarysource either way, so vet passes on
maintoo. Usego listto see whether theinode-marker test lands in the production
.GoFiles(bad) or in.TestGoFiles(correct):
Related issues
Use cases
N/A — test-only change; no runtime behavior is added or modified. It restores CI
coverage for filestream/file input paths (symlink handling was already covered
elsewhere; rename/remove/truncate reader closing and inode-marker identity were
not being exercised at all).
Screenshots
N/A
Logs
Reactivated tests passing under the race detector (linux/amd64):
This is an automatic backport of pull request #51853 done by [Mergify](https://mergify.com).