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 hang with ginkgo -p #1192

Merged
merged 4 commits into from
May 3, 2023
Merged
Show file tree
Hide file tree
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
60 changes: 60 additions & 0 deletions TODO
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Backlog
Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for merging, did you intent to commit the TODO file?

Copy link
Owner

Choose a reason for hiding this comment

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

no, sigh - i had just renamed it and updated my .gitignore on main but accidentally committed it in the pr. i've cleaned it up now, though it will live on forever in the git history. there's nothing sensitive in there

[] @G fix hang with ginkgo -p
https://github.com/onsi/ginkgo/issues/1192
[] @B biloba needs to support "McDonald's"
[] @G Document order of execution for setup nodes on the same level
https://github.com/onsi/ginkgo/issues/1172
[] @G fail fast may cause Serial spec or cleanup Node interrupted
https://github.com/onsi/ginkgo/issues/1178

# Needs-Prioritization
[] @G Pull JUnit config out of code and into flags/config files
https://github.com/onsi/ginkgo/issues/1115
[] @G -exec support for `ginkgo run`
https://github.com/onsi/ginkgo/issues/869
[] @G Add `indent` to gcustom
[] @G HaveField should not panic in FailureMessage if the field is non-existant. It should return a meaningful failure message.
[] @G allow NodeTimeout and GracePeriod to live on containers and propagate down
[] @G Clean up ReportEntry decoding:
func (entry ReportEntry) DecodeValue(p interface{}) error {
// if raw is assignable to p, assign
// else - parse AsJSON
}
[] @B Biloba lets you get all innerHTML and emit it on failure
[] @B equivalent of puppeteer text selector?
[] @B how do we invoke async functions? what does await look like for those? maybe time to actually read? goal: remove the separate muhasibPostLogin function.
[] @B https://github.com/onsi/biloba/issues/2
[] @B right now polling an element fails if the browser if ollowing redirects. so i'm using Eventually(b.Location) - instead of just Eventually("#el").Should(b.Exist()). I think we need a more robust way to ensure biloba.
[] @B biloba support for gettign "SelectedOption" instead of just "Value" for select inputs (e.g. b.OptionByName(...) instead of value?)
[] @B ginkgo interrupt should not show the whole stacktrace. it's just too much!
[] @B add cookie support
chromedp.Run(b.Context, chromedp.ActionFunc(func(ctx context.Context) error {
return storage.ClearDataForOrigin("*", "all").Do(ctx)
}))

chromedp.Run(b.Context, chromedp.ActionFunc(func(ctx context.Context) error {
expr := cdp.TimeSinceEpoch(time.Date(2091, 28, 3, 1, 40, 45, 0, time.UTC))
return storage.SetCookies([]*network.CookieParam{{
Name: "rallly-session",
Value: "Fe26.2*1*66a5cae1dd8728fc7be37a1a3c485557606e526b16b472329be78168ad4d48c2*Yb_O9pN2K3APF6LXt9S3zg*IZQ_c5aukJzt-AIW__lL19igVhpFMGH9cK0PyFenF-2ti94BgBsLDf325DB2rsKE*3825906104968*a06f3cfe6ef65db1a30b5177cb767c914ca38c8fc3e2456de89d5bea5641611e*6HNCfDeEzgfeQO88IRJ8TfdG5IDzDQtt6WaoGAg5i98~2", Expires: &expr,
Domain: "rallly.co",
Path: "/",
HTTPOnly: true,
Secure: true,
SameSite: network.CookieSameSiteLax,
}}).WithBrowserContextID(b.BrowserContextID()).Do(ctx)
}))
[] @Ω Gomega should have an error returning mode, then tell pohly
[] @Ω Gomega submatcher description interface and bare-element interface (the former for any sort of matcher that takes a submatcher; the latter specifically for matchers like Consistently etc. that would replace equalMatchersToElements.


[] @B JSSelector (is a function that returns one or many nodes)
[] @B ScrollTo etc.
[] @B support for esbuild? or something? consider the auth_login_scripts tests - what might make them better?

# Long-term Backlog
- VSCode Extension
- Ginkgo WebView
- Suite configuration
- Suite parallelization
- With the special subcase of supporting shared singleton resources
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package main_test

import (
"fmt"
"os"
"os/exec"
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestInterceptorSleepFixture(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "TestInterceptorSleepFixture Suite")
}

var _ = Describe("Ensuring that ginkgo -p does not hang when output is intercepted", func() {
It("ginkgo -p should not hang on this spec", func() {
fmt.Fprintln(os.Stdout, "Some STDOUT output")
fmt.Fprintln(os.Stderr, "Some STDERR output")
cmd := exec.Command("sleep", "60")
Ω(cmd.Start()).Should(Succeed())
})
})
18 changes: 18 additions & 0 deletions integration/output_interceptor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package integration_test

import (
"os/exec"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/gbytes"
"github.com/onsi/gomega/gexec"
)

Expand Down Expand Up @@ -47,4 +49,20 @@ var _ = Describe("OutputInterceptor", func() {
Ω(report.SpecReports[0].CapturedStdOutErr).Should(Equal("CAPTURED OUTPUT A\nCAPTURED OUTPUT B\n"))
})
})

Context("ensuring Ginkgo does not hang when a child process does not exit: https://github.com/onsi/ginkgo/issues/1191", func() {
BeforeEach(func() {
fm.MountFixture("interceptor_sleep")
})

It("exits without hanging", func() {
sess := startGinkgo(fm.PathTo("interceptor_sleep"), "--no-color", "--procs=2")
Eventually(sess).WithTimeout(time.Second * 15).Should(gexec.Exit(0))

Ω(sess).Should(gbytes.Say("Captured StdOut/StdErr Output >>"))
Ω(sess).Should(gbytes.Say("Some STDOUT output"))
Ω(sess).Should(gbytes.Say("Some STDERR output"))
Ω(sess).Should(gbytes.Say("<< Captured StdOut/StdErr Output"))
})
})
})
11 changes: 11 additions & 0 deletions internal/output_interceptor_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@ func (impl *dupSyscallOutputInterceptorImpl) CreateStdoutStderrClones() (*os.Fil
stdoutCloneFD, _ := unix.Dup(1)
stderrCloneFD, _ := unix.Dup(2)

// Important, set the fds to FD_CLOEXEC to prevent them leaking into childs
// https://github.com/onsi/ginkgo/issues/1191
flags, err := unix.FcntlInt(uintptr(stdoutCloneFD), unix.F_GETFD, 0)
if err == nil {
unix.FcntlInt(uintptr(stdoutCloneFD), unix.F_SETFD, flags|unix.FD_CLOEXEC)
}
flags, err = unix.FcntlInt(uintptr(stderrCloneFD), unix.F_GETFD, 0)
if err == nil {
unix.FcntlInt(uintptr(stderrCloneFD), unix.F_SETFD, flags|unix.FD_CLOEXEC)
}

// And then wrap the clone file descriptors in files.
// One benefit of this (that we don't use yet) is that we can actually write
// to these files to emit output to the console even though we're intercepting output
Expand Down