Closed m-kostrzewa closed 6 years ago
Thanks for the PR, overally I like the idea of making it more testable. (I have some nitpicks, though)
One thing I'm not sure about is that we're no longer emit messages online. We could make Emitter
a trait instead, with RealEmitter
and TestEmitter
instead. What do you think? (This way we also won't have to iterate over the vec in main
).
Regarding testing, I think that in addition to "full run" tests as you described, we could also test the parse
and analyze
method separately. (That would require splitting parse
to parse_file
and parse_string
)
In order to write tests, we could implement a test version of emit() method and assert like
In order to do that we need to be able to inject some kind of FakeEmitter that has its own emit() method. It's because the "old" emitter just prints everything to stdout, so we can't easily create assertions.
Changes:
This code needs polishing and can be refactored further, but I wanted to see your comments before continuing. Especially since I'm a Rust noob.