Skip to content
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 hack/e2e.go exit status for -test #4339

Merged
merged 2 commits into from
Feb 11, 2015
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 11 additions & 44 deletions hack/e2e.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,21 +146,21 @@ func main() {
}
}

failure := false
success := true
switch {
case *ctlCmd != "":
ctlArgs := strings.Fields(*ctlCmd)
os.Setenv("KUBE_CONFIG_FILE", "config-test.sh")
failure = !finishRunning("'kubectl "+*ctlCmd+"'", exec.Command(path.Join(versionRoot, "cluster/kubectl.sh"), ctlArgs...))
success = finishRunning("'kubectl "+*ctlCmd+"'", exec.Command(path.Join(versionRoot, "cluster/kubectl.sh"), ctlArgs...))
case *test:
failure = Test()
success = Test()
}

if *down {
TearDown()
}

if failure {
if !success {
os.Exit(1)
}
}
Expand Down Expand Up @@ -277,8 +277,8 @@ func Test() bool {
// call the returned anonymous function to stop.
func runBashUntil(stepName string, cmd *exec.Cmd) func() {
log.Printf("Running in background: %v", stepName)
stdout, stderr := bytes.NewBuffer(nil), bytes.NewBuffer(nil)
cmd.Stdout, cmd.Stderr = stdout, stderr
output := bytes.NewBuffer(nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we could simply use cmd.CombinedOutput() here? Maybe in a goroutine? It's not like we're streaming the output to stdout live, we're just capturing it to include in the final output... Not 100% sure if it will work with a command that gets interrupted but I think it just might... What do you think?

cmd.Stdout, cmd.Stderr = output, output
if err := cmd.Start(); err != nil {
log.Printf("Unable to start '%v': '%v'", stepName, err)
return func() {}
Expand All @@ -287,7 +287,7 @@ func runBashUntil(stepName string, cmd *exec.Cmd) func() {
cmd.Process.Signal(os.Interrupt)
headerprefix := stepName + " "
lineprefix := " "
printBashOutputs(headerprefix, lineprefix, string(stdout.Bytes()), string(stderr.Bytes()), false)
printBashOutputs(headerprefix, lineprefix, string(output.Bytes()), false)
}
}

Expand Down Expand Up @@ -327,44 +327,11 @@ func finishRunning(stepName string, cmd *exec.Cmd) bool {
return result
}

func printBashOutputs(headerprefix, lineprefix, stdout, stderr string, escape bool) {
// The |'s (plus appropriate prefixing) are to make this look
// "YAMLish" to the Jenkins TAP plugin:
// https://wiki.jenkins-ci.org/display/JENKINS/TAP+Plugin
if stdout != "" {
fmt.Printf("%vstdout: |\n", headerprefix)
if escape {
stdout = escapeOutput(stdout)
}
printPrefixedLines(lineprefix, stdout)
}
if stderr != "" {
fmt.Printf("%vstderr: |\n", headerprefix)
if escape {
stderr = escapeOutput(stderr)
}
printPrefixedLines(lineprefix, stderr)
}
}

// Escape stdout/stderr so the Jenkins YAMLish parser doesn't barf on
// it. This escaping is crude (it masks all colons as something humans
// will hopefully see as a colon, for instance), but it should get the
// job done without pulling in a whole YAML package.
func escapeOutput(s string) (out string) {
for _, r := range s {
switch {
case r == '\n':
out += string(r)
case !strconv.IsPrint(r):
out += " "
case r == ':':
out += "\ua789" // "꞉", modifier letter colon
default:
out += string(r)
}
func printBashOutputs(headerprefix, lineprefix, output string, escape bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this but it's unrelated to the exit status of -test, so can we please keep it in a separate commit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those hunks are easy to separate, splitting.

if output != "" {
fmt.Printf("%voutput: |\n", headerprefix)
printPrefixedLines(lineprefix, output)
}
return
}

func printPrefixedLines(prefix, s string) {
Expand Down