From edbc1d3b4aa430430dc3c4cab3f3c324dd7b4fd9 Mon Sep 17 00:00:00 2001 From: lsytj0413 <511121939@qq.com> Date: Fri, 10 Apr 2020 13:36:36 +0800 Subject: [PATCH 1/2] test(*): TestRollover failed randomly on Windows --- klog_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/klog_test.go b/klog_test.go index 404c058f..a298b739 100644 --- a/klog_test.go +++ b/klog_test.go @@ -28,6 +28,7 @@ import ( "runtime" "strconv" "strings" + "sync" "testing" "time" ) @@ -317,13 +318,16 @@ func TestVmoduleOff(t *testing.T) { func TestSetOutputDataRace(t *testing.T) { setFlags() defer logging.swap(logging.newBuffers()) + var wg sync.WaitGroup for i := 1; i <= 50; i++ { go func() { logging.flushDaemon() }() } for i := 1; i <= 50; i++ { + wg.Add(1) go func() { + defer wg.Done() SetOutput(ioutil.Discard) }() } @@ -333,7 +337,9 @@ func TestSetOutputDataRace(t *testing.T) { }() } for i := 1; i <= 50; i++ { + wg.Add(1) go func() { + defer wg.Done() SetOutputBySeverity("INFO", ioutil.Discard) }() } @@ -342,6 +348,7 @@ func TestSetOutputDataRace(t *testing.T) { logging.flushDaemon() }() } + wg.Wait() } // vGlobs are patterns that match/don't match this file at V=2. From 5727d2aa060c7b678d18b57c3e977e63ebfb1c6f Mon Sep 17 00:00:00 2001 From: Sebastian Soto Date: Thu, 9 Apr 2020 14:17:31 -0400 Subject: [PATCH 2/2] Fix Windows integration tests This commit fixes issues with Windows integration tests and enables the tests to run through github. The problem was that the tests using the log_dir option expected symlinks to the log files to exist, those symlinks being (main.INFO, main.WARN, etc). This was a problem for two reasons: 1) On Windows the symlinks would start with main.exe, not just main 2) The creation of symlinks is not guarenteed on any platform, and on Windows creating a symlink requires Administrator permissions. This was fixed by changing the tests to search for either the symlinks (with a fixed filepath), or the log files themselves. Future work can be done to deal with the lack of symlinks on Windows, but this commit's purpose is to get tests in a passing state. --- .github/workflows/test.yml | 2 +- integration_tests/klog_test.go | 59 ++++++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 82ba8d38..ec3a0eec 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,7 +5,7 @@ jobs: strategy: matrix: go-versions: [1.12.x, 1.13.x, 1.14.x] - platform: [ubuntu-latest, macos-latest] + platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} steps: - name: Install Go diff --git a/integration_tests/klog_test.go b/integration_tests/klog_test.go index ef70fb67..6c515e69 100644 --- a/integration_tests/klog_test.go +++ b/integration_tests/klog_test.go @@ -2,12 +2,15 @@ package integration_tests_test import ( "bytes" + "fmt" "io" "io/ioutil" "os" "os/exec" "path/filepath" "regexp" + "runtime" + "strings" "testing" ) @@ -181,6 +184,11 @@ func TestDestinationsWithDifferentFlags(t *testing.T) { }, } + binaryFileExtention := "" + if runtime.GOOS == "windows" { + binaryFileExtention = ".exe" + } + for tcName, tc := range tests { tc := tc t.Run(tcName, func(t *testing.T) { @@ -214,13 +222,19 @@ func TestDestinationsWithDifferentFlags(t *testing.T) { } // check files in log_dir - for level, file := range logFileName { - logfile := filepath.Join(logdir, file) // /some/tmp/dir/main.WARNING + for level, levelName := range logFileLevels { + binaryName := "main" + binaryFileExtention + logfile, err := getLogFilePath(logdir, binaryName, levelName) if tc.expectedLogDir { + if err != nil { + t.Errorf("Unable to find log file: %v", err) + } content := getFileContent(t, logfile) - checkForLogs(t, tc.expectedInDir[level], tc.notExpectedInDir[level], content, "logfile["+file+"]") + checkForLogs(t, tc.expectedInDir[level], tc.notExpectedInDir[level], content, "logfile["+logfile+"]") } else { - assertFileIsAbsent(t, logfile) + if err == nil { + t.Errorf("Unexpectedly found log file %s", logfile) + } } } }) @@ -252,11 +266,11 @@ func klogRun(t *testing.T, flags []string, stderr io.Writer) { } } -var logFileName = map[int]string{ - 0: "main.INFO", - 1: "main.WARNING", - 2: "main.ERROR", - 3: "main.FATAL", +var logFileLevels = map[int]string{ + 0: "INFO", + 1: "WARNING", + 2: "ERROR", + 3: "FATAL", } func getFileContent(t *testing.T, filePath string) string { @@ -306,3 +320,30 @@ func withTmpDir(t *testing.T, f func(string)) { f(tmpDir) } + +// getLogFileFromDir returns the path of either the symbolic link to the logfile, or the the logfile itself. This must +// be done as the creation of a symlink is not guaranteed on any platform. On Windows, only users with administration +// privileges can create a symlink. +func getLogFilePath(dir, binaryName, levelName string) (string, error) { + symlink := filepath.Join(dir, binaryName+"."+levelName) + if _, err := os.Stat(symlink); err == nil { + return symlink, nil + } + files, err := ioutil.ReadDir(dir) + if err != nil { + return "", fmt.Errorf("could not read directory %s: %v", dir, err) + } + var foundFile string + for _, file := range files { + if strings.HasPrefix(file.Name(), binaryName) && strings.Contains(file.Name(), levelName) { + if foundFile != "" { + return "", fmt.Errorf("found multiple matching files") + } + foundFile = file.Name() + } + } + if foundFile != "" { + return filepath.Join(dir, foundFile), nil + } + return "", fmt.Errorf("file missing from directory") +}