Skip to content

Commit

Permalink
include cancellation reason when cancelling spec context
Browse files Browse the repository at this point in the history
  • Loading branch information
onsi committed Jan 17, 2024
1 parent 4fcd0b3 commit 96e915c
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 5 deletions.
27 changes: 27 additions & 0 deletions internal/internal_integration/interrupt_and_timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
select {
case <-c.Done():
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
F("bam")
case <-time.After(time.Hour):
}
Expand Down Expand Up @@ -236,6 +237,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
select {
case <-c.Done():
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
time.Sleep(time.Hour)
case <-time.After(time.Hour):
}
Expand Down Expand Up @@ -275,6 +277,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
select {
case <-c.Done():
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
time.Sleep(time.Millisecond * 100)
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
time.Sleep(time.Hour)
Expand Down Expand Up @@ -325,12 +328,14 @@ var _ = Describe("Interrupts and Timeouts", func() {
It("A", rt.TSC("A", func(c SpecContext) {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
<-c.Done()
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
}))
It("B", rt.T("B"))
AfterEach(rt.TSC("aft-inner", func(c SpecContext) {
t := time.Now()
select {
case <-c.Done():
Ω(context.Cause(c)).Should(MatchError("grace period timeout occurred"))
times.Set("aft-inner", time.Since(t))
time.Sleep(time.Second)
case <-time.After(time.Second):
Expand Down Expand Up @@ -380,11 +385,13 @@ var _ = Describe("Interrupts and Timeouts", func() {
It("A", rt.TSC("A", func(c SpecContext) {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
<-c.Done()
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
}))
It("B", rt.T("B"))
AfterEach(rt.TSC("aft-inner", func(c SpecContext) {
select {
case <-c.Done():
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set("aft-inner", time.Since(t))
time.Sleep(time.Second)
case <-time.After(time.Second):
Expand Down Expand Up @@ -432,11 +439,13 @@ var _ = Describe("Interrupts and Timeouts", func() {
It("A", rt.TSC("A", func(c SpecContext) {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
<-c.Done()
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
}))
It("B", rt.T("B"))
AfterEach(rt.TSC("aft-inner", func(c SpecContext) {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
<-c.Done()
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
}))
})
AfterEach(rt.T("aft-outer"))
Expand Down Expand Up @@ -479,11 +488,13 @@ var _ = Describe("Interrupts and Timeouts", func() {
It("A", rt.TSC("A", func(c SpecContext) {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
<-c.Done()
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
}))
It("B", rt.T("B"))
AfterEach(rt.TSC("aft-inner", func(c SpecContext) {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
<-c.Done()
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
}))
})
AfterEach(rt.T("aft-outer"))
Expand Down Expand Up @@ -536,6 +547,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
BeforeEach(rt.TSC("bef-inner", func(c SpecContext) {
interruptHandler.Interrupt(interrupt_handler.InterruptCauseSignal)
<-c.Done()
Ω(context.Cause(c)).Should(MatchError(interrupt_handler.InterruptCauseSignal.String()))
}))

Context("even more nested", func() {
Expand Down Expand Up @@ -629,6 +641,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
Describe("when it exits in time", func() {
It("A", rt.TSC("A", func(c SpecContext) {
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
rt.Run("A-cancelled")
Fail("subsequent failure message")
}), NodeTimeout(time.Millisecond*100))
Expand All @@ -637,13 +650,15 @@ var _ = Describe("Interrupts and Timeouts", func() {
Describe("with no configured grace period", func() {
It("B", rt.TSC("B", func(c SpecContext) {
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
time.Sleep(time.Hour)
}), NodeTimeout(time.Millisecond*100))
})

Describe("with a configured grace period", func() {
It("C", rt.TSC("C", func(c SpecContext) {
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
time.Sleep(time.Hour)
}), NodeTimeout(time.Millisecond*100), GracePeriod(time.Millisecond*50))
})
Expand Down Expand Up @@ -714,6 +729,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
}, func(c SpecContext, b []byte) {
rt.Run(string(b))
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set(string(b), time.Since(t))
}, NodeTimeout(time.Millisecond*100))

Expand All @@ -724,6 +740,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
}, func(c SpecContext) {
rt.Run("afts-proc-1")
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set("afts-proc-1", time.Since(t))
}, NodeTimeout(time.Millisecond*200))
})
Expand Down Expand Up @@ -763,6 +780,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
AfterEach(rt.TSC("aft-1", func(c SpecContext) {
times.Set("A", time.Since(t))
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("grace period timeout occurred"))
times.Set("aft-1-cancel", time.Since(t))
writer.Println("aft-1")
time.Sleep(time.Hour)
Expand All @@ -771,6 +789,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
AfterEach(rt.TSC("aft-2", func(c SpecContext) {
times.Set("aft-1-out", time.Since(t))
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("grace period timeout occurred"))
times.Set("aft-2-cancel", time.Since(t))
writer.Println("aft-2")
time.Sleep(time.Hour)
Expand All @@ -779,6 +798,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
AfterEach(rt.TSC("aft-3", func(c SpecContext) {
times.Set("aft-2-out", time.Since(t))
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set("aft-3-cancel", time.Since(t))
writer.Println("aft-3")
time.Sleep(time.Hour)
Expand Down Expand Up @@ -839,6 +859,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
BeforeEach(rt.TSC("bef-A", func(c SpecContext) {
t := time.Now()
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set("bef-A", time.Since(t))
}), NodeTimeout(time.Millisecond*100))

Expand All @@ -849,6 +870,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
BeforeEach(rt.TSC("bef-B", func(c SpecContext) {
t := time.Now()
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("spec timeout occurred"))
times.Set("bef-B", time.Since(t))
}), NodeTimeout(time.Millisecond*250))

Expand All @@ -859,6 +881,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
BeforeEach(rt.TSC("bef-C", func(c SpecContext) {
t := time.Now()
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("suite timeout occurred"))
times.Set("bef-C", time.Since(t))
}), NodeTimeout(time.Millisecond*300))

Expand Down Expand Up @@ -914,6 +937,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
Context("container", func() {
It("A", rt.TSC("A", func(c SpecContext) {
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("suite timeout occurred"))
}))

It("B", rt.T("B"))
Expand Down Expand Up @@ -947,13 +971,15 @@ var _ = Describe("Interrupts and Timeouts", func() {
rt.Run(key)
t := time.Now()
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set(key, time.Since(t))
}, NodeTimeout(time.Millisecond*100), "dc-1")

DeferCleanup(func(c SpecContext, key string) {
rt.Run(key)
t := time.Now()
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set(key, time.Since(t))
}, NodeTimeout(time.Millisecond*100), "dc-2")

Expand All @@ -962,6 +988,7 @@ var _ = Describe("Interrupts and Timeouts", func() {
rt.Run(key)
t := time.Now()
<-c.Done()
Ω(context.Cause(c)).Should(MatchError("node timeout occurred"))
times.Set(key, time.Since(t))
}, NodeTimeout(time.Millisecond*100), context.WithValue(context.Background(), "key", "dc"), "-3")

Expand Down
4 changes: 2 additions & 2 deletions internal/spec_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type specContext struct {
context.Context
*ProgressReporterManager

cancel context.CancelFunc
cancel context.CancelCauseFunc

suite *Suite
}
Expand All @@ -30,7 +30,7 @@ Note that while SpecContext is used to enforce deadlines by Ginkgo it is not con
This is because Ginkgo needs finer control over when the context is canceled. Specifically, Ginkgo needs to generate a ProgressReport before it cancels the context to ensure progress is captured where the spec is currently running. The only way to avoid a race here is to manually control the cancellation.
*/
func NewSpecContext(suite *Suite) *specContext {
ctx, cancel := context.WithCancel(context.Background())
ctx, cancel := context.WithCancelCause(context.Background())
sc := &specContext{
cancel: cancel,
suite: suite,
Expand Down
6 changes: 3 additions & 3 deletions internal/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,7 +858,7 @@ func (suite *Suite) runNode(node Node, specDeadline time.Time, text string) (typ
}

sc := NewSpecContext(suite)
defer sc.cancel()
defer sc.cancel(fmt.Errorf("spec has finished"))

suite.selectiveLock.Lock()
suite.currentSpecContext = sc
Expand Down Expand Up @@ -958,7 +958,7 @@ func (suite *Suite) runNode(node Node, specDeadline time.Time, text string) (typ

// tell the spec to stop. it's important we generate the progress report first to make sure we capture where
// the spec is actually stuck
sc.cancel()
sc.cancel(fmt.Errorf("%s timeout occurred", timeoutInPlay))
//and now we wait for the grace period
gracePeriodChannel = time.After(gracePeriod)
case <-interruptStatus.Channel:
Expand All @@ -985,7 +985,7 @@ func (suite *Suite) runNode(node Node, specDeadline time.Time, text string) (typ
}

progressReport = progressReport.WithoutOtherGoroutines()
sc.cancel()
sc.cancel(fmt.Errorf(interruptStatus.Message()))

if interruptStatus.Level == interrupt_handler.InterruptLevelBailOut {
if interruptStatus.ShouldIncludeProgressReport() {
Expand Down

0 comments on commit 96e915c

Please sign in to comment.