testdriver: improve debug logging #236
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
milesbxf
force-pushed
the
testdriver-debug-logging
branch
2 times, most recently
from
64fb247
to
e27c545
Compare
milesbxf
force-pushed
the
testdriver-debug-logging
branch
2 times, most recently
from
71ac8b7
to
a475c1a
Compare
myitcv
changed the title
|
One small request: with commits, PRs and issues, please can you follow the following approach? https://golang.org/doc/contribute.html#commit_messages Specifically:
For example, here are the commits in https://github.com/myitcv/govim/commits/master It just means it's then trivial to create a relatively brief log of what's happened between versions: |
milesbxf
force-pushed
the
testdriver-debug-logging
branch
7 times, most recently
from
f96806f
to
24bade2
Compare
milesbxf
changed the title
myitcv
reviewed
Jun 17, 2019
govim_test.go
Outdated
| if err != nil { | ||
| t.Fatalf("failed to create new driver: %v", err) | ||
| } | ||
| fmt.Printf("Vim home path is at %s\n", home) |
There was a problem hiding this comment.
These lines make the output of go test very noisy, which doesn't make much sense because it ends up outputting one of these lines per testscript. e.g. in the module root:
$ go test
Vim home path is at /tmp/go-test-script913401979/script-autocmd/home
Vim home path is at /tmp/go-test-script913401979/script-feedkeys_working/home
Vim home path is at /tmp/go-test-script913401979/script-redraw/home
Vim home path is at /tmp/go-test-script913401979/script-version/home
Vim home path is at /tmp/go-test-script913401979/script-range_function/home
Vim home path is at /tmp/go-test-script913401979/script-normal/home
Vim home path is at /tmp/go-test-script913401979/script-unscheduled/home
Vim home path is at /tmp/go-test-script913401979/script-function/home
Running testdriver for script-version
Running testdriver for script-function
Running testdriver for script-unscheduled
Running testdriver for script-normal
Running testdriver for script-range_function
Running testdriver for script-redraw
Running testdriver for script-feedkeys_working
Running testdriver for script-autocmd
Testdriver init finished for script-function
Setup complete for script-function
Testdriver init finished for script-version
Setup complete for script-version
Testdriver init finished for script-normal
Setup complete for script-normal
Testdriver init finished for script-unscheduled
Setup complete for script-unscheduled
Testdriver init finished for script-redraw
Setup complete for script-redraw
Testdriver init finished for script-feedkeys_working
Setup complete for script-feedkeys_working
Testdriver init finished for script-autocmd
Setup complete for script-autocmd
Testdriver init finished for script-range_function
Setup complete for script-range_function
Vim home path is at /tmp/go-test-script913401979/script-condition_vim/home
Vim home path is at /tmp/go-test-script913401979/script-feedkeys_broken/home
Vim home path is at /tmp/go-test-script913401979/script-expr/home
Vim home path is at /tmp/go-test-script913401979/script-ex/home
Running testdriver for script-condition_vim
Running testdriver for script-feedkeys_broken
Vim home path is at /tmp/go-test-script913401979/script-call/home
Vim home path is at /tmp/go-test-script913401979/script-command/home
Vim home path is at /tmp/go-test-script913401979/script-bad_function/home
Running testdriver for script-ex
Running testdriver for script-expr
Testdriver init finished for script-feedkeys_broken
Setup complete for script-feedkeys_broken
Testdriver init finished for script-condition_vim
Setup complete for script-condition_vim
Running testdriver for script-command
Running testdriver for script-call
Running testdriver for script-bad_function
Testdriver init finished for script-expr
Setup complete for script-expr
Testdriver init finished for script-ex
Setup complete for script-ex
Testdriver init finished for script-call
Setup complete for script-call
Testdriver init finished for script-bad_function
Setup complete for script-bad_function
Testdriver init finished for script-command
Setup complete for script-command
Would it not make sense to make this debug lines, and only have them output with -debugLog?
Could create a logf function or similar to keep things succinct
There was a problem hiding this comment.
Yes, very good point - I'll go through and vet the printf calls
| @@ -60,7 +63,22 @@ type TestDriver struct { | |||
| closed bool | |||
| } | |||
|
|
|||
| func NewTestDriver(name string, govimPath, testHomePath, testPluginPath string, env *testscript.Env, plug govim.Plugin) (*TestDriver, error) { | |||
| type Config struct { | |||
testdriver/testdriver.go
Outdated
| func (d *TestDriver) LogStripANSI(r io.Reader) { | ||
| buf := bufio.NewReader(r) | ||
| for { | ||
| l, err := buf.ReadString('\n') |
There was a problem hiding this comment.
Beware! In #299 I effectively fell foul of this default bufio buffer overflowing because of long lines. What do you actually use this output for?
There was a problem hiding this comment.
Ah, good catch, though I haven't come across this in testing.
I've found this very useful when debugging Vim startup errors
|
|
||
| return res, nil | ||
| } | ||
|
|
||
| func (d *TestDriver) Logf(format string, a ...interface{}) { | ||
| fmt.Fprintf(d.Log, format+"\n", a...) |
There was a problem hiding this comment.
Again, only output if debugging enabled?
There was a problem hiding this comment.
This should only be going to d.Log, which by default only logs to an error file
milesbxf
force-pushed
the
testdriver-debug-logging
branch
from
24bade2
to
c473ed0
Compare
This makes it easier to debug issues with the govim tests, by allowing users to log more information with a general -debugLog flag. This enables Vim debug logging, and sends Vim startup output to the normal Govim log. If Vim fails to start, it prints out the contents of the log, which should hopefully contain some useful information. It also prints a full command to run Vim with the temporary home directory. Since the call to NewTestDriver was getting a bit unwieldy with over 10 args, I refactored that out into a Config struct.
milesbxf
force-pushed
the
testdriver-debug-logging
branch
from
c473ed0
to
9dbefb2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes it easier to debug issues with the govim tests, by allowing
users to log more information with a general -debugLog flag. This
enables Vim debug logging, and sends Vim startup output to the normal
Govim log. If Vim fails to start, it prints out the contents of the log,
which should hopefully contain some useful information. It also prints a
full command to run Vim with the temporary home directory.
Since the call to NewTestDriver was getting a bit unwieldy with over 10
args, I refactored that out into a Config struct.