- 
                Notifications
    You must be signed in to change notification settings 
- Fork 69
Snapshot test the terminal UI #400
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
Conversation
| column int | ||
| } | ||
|  | ||
| func (t *ttyBuf) Write(b []byte) (int, error) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
| btw. I know that "test coverage" itself doesn't mean a lot, but with the latest commit this bumps the test coverage for  | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hideous. I love it. ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool!
* Hacky integration test for CampaignProgressPrinter * Clean up ttyBuf * Force width and height in integration test * For now: skip on Windows * Add completed diff to progress printer test * Print again to make sure that the output is the same * Sort filenames before printing diff summary * Fix missing clearCurrentLine call * Always create a new line on newline character * Be more defensive in ttyBuf * Use stable timestamps in test * Clean up duplicated default opts * Make handling of no-spinner consistent
I started with one thought: "I just wish I had one test to make sure that this whole thing [the terminal UI] doesn't blow up every time I make a change" What I ended with is this. It includes a tiny VT100 "emulator".
And here's the kicker: I actually like this. It's fast to run and easy to extend and it gives me confidence that the basic rendering functionality works. Of course, the hacky injections need to be cleaned up before merging. And maybe we can get it to pass on Windows.
Opinions?
(This fixes https://github.com/sourcegraph/sourcegraph/issues/16330)