Skip to content

Commit

Permalink
fix quickfix list when test duration exceeds timeout
Browse files Browse the repository at this point in the history
Do not treat a panic caused by a test timeout as an error that occurred
within a test's callstack. When go test panics due to a test timeout,
the running goroutine's stacktrace only contains entries for the
standard library, and there is one goroutine whose stacktrace contains
entries for the standard library and the test binary. None of the other
goroutines should be considered to contain an error, either, because the
panic was not due to any code executing within those goroutines' stacks.
Therefore, treat panics caused by a test timeout as an error without a
file location.
  • Loading branch information
bhcleek committed Dec 31, 2017
1 parent bb99031 commit 4895313
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 0 deletions.
47 changes: 47 additions & 0 deletions autoload/go/test-fixtures/test/src/timeout/timeout_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Run a few parallel tests, all in parallel, using multiple techniques for
// causing the test to take a while so that the stacktraces resulting from a
// test timeout will contain several goroutines to avoid giving a false sense
// of confidence or creating error formats that don't account for the more
// complex scenarios that can occur with timeouts.

package main

import (
"testing"
"time"
)

func TestSleep(t *testing.T) {
t.Parallel()
time.Sleep(15 * time.Second)
t.Log("expected panic if run with timeout < 15s")
}

func TestRunning(t *testing.T) {
t.Parallel()
c := time.After(15 * time.Second)
Loop:
for {
select {
case <-c:
break Loop
default:
}
}

t.Log("expected panic if run with timeout < 15s")
}

func TestRunningAlso(t *testing.T) {
t.Parallel()
c := time.After(15 * time.Second)
Loop:
for {
select {
case <-c:
break Loop
default:
}
}
t.Log("expected panic if run with timeout < 15s")
}
9 changes: 9 additions & 0 deletions autoload/go/test.vim
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,10 @@ function! s:errorformat() abort

" set the format for panics.

" handle panics from test timeouts
let format .= ",%+Gpanic: test timed out after %.%\\+"

" handle non-timeout panics
" In addition to 'panic', check for 'fatal error' to support older versions
" of Go that used 'fatal error'.
"
Expand Down Expand Up @@ -368,6 +372,11 @@ function! s:errorformat() abort
" library line in stack traces.
let format .= ",%-G" . readyaddress

" a blank line is the end of a multi-line message. This is only relevant
" when tests panic; the goroutine stacktrace sections are separated by an
" empty line.
let format .= ",%Z%^%$"

" Match and ignore exit status lines (produced when go test panics) whether
" part of a multi-line message or not, because these lines sometimes come
" before and sometimes after panic stacktraces.
Expand Down
10 changes: 10 additions & 0 deletions autoload/go/test_test.vim
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ func! Test_GoTestCompilerError() abort
call s:test('compilerror/compilerror_test.go', expected)
endfunc

func! Test_GoTestTimeout() abort
let expected = [
\ {'lnum': 0, 'bufnr': 0, 'col': 0, 'valid': 1, 'vcol': 0, 'nr': -1, 'type': '', 'pattern': '', 'text': 'panic: test timed out after 2s'}
\ ]

let g:go_test_timeout="2s"
call s:test('timeout/timeout_test.go', expected)
unlet g:go_test_timeout
endfunc

func! s:test(file, expected, ...) abort
if has('nvim')
" nvim mostly shows test errors correctly, but the the expected errors are
Expand Down

0 comments on commit 4895313

Please sign in to comment.