-
Notifications
You must be signed in to change notification settings - Fork 244
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.
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 odo log flake #3733
Fix odo log flake #3733
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,9 @@ package helper | |
import ( | ||
"bytes" | ||
"encoding/json" | ||
"errors" | ||
"fmt" | ||
"io" | ||
"os" | ||
"os/exec" | ||
"path/filepath" | ||
|
@@ -177,6 +179,66 @@ func WatchNonRetCmdStdOut(cmdStr string, timeout time.Duration, check func(outpu | |
} | ||
} | ||
|
||
// RunCmdWithMatchOutputFromBuffer starts the command, and command stdout is attached to buffer. | ||
// we read data from buffer line by line, and if expected string is matched it returns true | ||
// It is different from WaitforCmdOut which gives stdout in one go using session.Out.Contents() | ||
// for commands like odo log -f which streams continuous data and does not terminate by their own | ||
// we need to read the stream data from buffer. | ||
func RunCmdWithMatchOutputFromBuffer(timeoutAfter time.Duration, matchString, program string, args ...string) (bool, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking instead of overloading helper file, Can't we use gingko in-build pattern for timeout - https://onsi.github.io/ginkgo/#asynchronous-tests There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @prietyc123 I follow the pattern from
mik-dass marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var buf bytes.Buffer | ||
|
||
command := exec.Command(program, args...) | ||
command.Stdout = &buf | ||
command.Stderr = &buf | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should also check the StdErr stream. In case of the timeout, we can print the contents from the error buffer. It will help in debugging in case the command fails to stream. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to use a different buffer for the error stream like in case of the watch test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
timeoutCh := time.After(timeoutAfter) | ||
matchOutputCh := make(chan bool) | ||
errorCh := make(chan error) | ||
|
||
_, err := fmt.Fprintln(GinkgoWriter, runningCmd(command)) | ||
if err != nil { | ||
return false, err | ||
} | ||
|
||
err = command.Start() | ||
if err != nil { | ||
return false, err | ||
} | ||
|
||
// go routine which is reading data from buffer until expected string matched | ||
go func() { | ||
for { | ||
line, err := buf.ReadString('\n') | ||
if err != nil && err != io.EOF { | ||
errorCh <- err | ||
} | ||
if len(line) > 0 { | ||
_, err = fmt.Fprintln(GinkgoWriter, line) | ||
if err != nil { | ||
errorCh <- err | ||
} | ||
if strings.Contains(line, matchString) { | ||
matchOutputCh <- true | ||
} | ||
} | ||
} | ||
}() | ||
|
||
for { | ||
select { | ||
case <-timeoutCh: | ||
fmt.Fprintln(GinkgoWriter, buf.String()) | ||
return false, errors.New("Timeout waiting for the conditon") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no capitalization on errors |
||
case <-matchOutputCh: | ||
return true, nil | ||
case <-errorCh: | ||
fmt.Fprintln(GinkgoWriter, buf.String()) | ||
return false, <-errorCh | ||
} | ||
} | ||
|
||
} | ||
|
||
// GetUserHomeDir gets the user home directory | ||
func GetUserHomeDir() string { | ||
homeDir, err := os.UserHomeDir() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,9 +58,10 @@ var _ = Describe("odo devfile log command tests", func() { | |
output := helper.CmdShouldPass("odo", "log") | ||
Expect(output).To(ContainSubstring("ODO_COMMAND_RUN")) | ||
|
||
// test with follow flag | ||
output = helper.CmdShouldRunWithTimeout(1*time.Second, "odo", "log", "-f") | ||
Expect(output).To(ContainSubstring("program=devrun")) | ||
// Test odo log -f | ||
match, err := helper.RunCmdWithMatchOutputFromBuffer(30*time.Second, "program=devrun", "odo", "log", "-f") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
Expect(err).To(BeNil()) | ||
Expect(match).To(BeTrue()) | ||
|
||
}) | ||
|
||
|
@@ -83,8 +84,9 @@ var _ = Describe("odo devfile log command tests", func() { | |
Expect(output).To(ContainSubstring("ODO_COMMAND_DEBUG")) | ||
|
||
// test with follow flag | ||
output = helper.CmdShouldRunWithTimeout(1*time.Second, "odo", "log", "-f") | ||
Expect(output).To(ContainSubstring("program=debugrun")) | ||
match, err := helper.RunCmdWithMatchOutputFromBuffer(30*time.Second, "program=debugrun", "odo", "log", "-f") | ||
Expect(err).To(BeNil()) | ||
Expect(match).To(BeTrue()) | ||
|
||
}) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+100