extrawurst / gitui

Blazing 💥 fast terminal-ui for git written in rust 🦀
MIT License
18.64k stars 573 forks source link

Add snapshot test using insta #2411

Open cruessler opened 4 weeks ago

cruessler commented 4 weeks ago

@extrawurst This PR is another try at adding snapshot tests to gitui. It doesn’t render to SVG as some of the other approaches do, but it works with the existing App and draw and only requires a minor type change in order to get it to work. Also, the text-based snapshot is still reasonably easy to read. So this feels very promising.

Next, I want to try to write a test that has interactivity, be it loading data from an actual git repo or keyboard input from a user.

cruessler commented 3 weeks ago

@extrawurst I just marked this PR as ready for review! I found it way easier to create the test than I had anticipated, mostly because the application is already structured in a way that is very amenable to snapshot testing.

Known issues:

cruessler commented 2 weeks ago

@extrawurst Did you get a chance to have a look? :smile:

extrawurst commented 2 weeks ago

@cruessler ok i think this is a great prototype but i don't think its ready to be merged in this form.

  1. we should definitely make sure we can busyloop until a certain event happened (with a timeout) to make the test more stable.
  2. i am not a fan of it all living in main.rs what you pointed out would allow us to put this into a testing suite: putting what we have now in main on top of App into another layer, lets call it simply Gitui that we can then use in our tests with a proper public api.
cruessler commented 1 week ago

@extrawurst I’ve extracted app initialization as well as the main loop into a new struct, Gitui. main.rs is now a lot lighter, with the code and tests living in gitui.rs.

I’m not sure about how to best implement a busy loop, though. We might be able to send the event that we get here, through a channel to the caller. Then, we could, in the caller, have a recv_timeout that we could use to wait for specific events. Does that sound like a possible solution? (I’m only vaguely familiar with crossbeam, so I’m happy about any context that you might be able to share.)

extrawurst commented 6 days ago

@cruessler awesome!

through a channel to the caller

yeah lets do that. and please fix the clippy lints