linebender / norad

Rust crate for working with Unified Font Object files
Apache License 2.0
43 stars 12 forks source link

Experiment with test suite layout #152

Open madig opened 3 years ago

madig commented 3 years ago

I'm reading https://matklad.github.io//2021/05/31/how-to-test.html and it contains several things that look like they're worth exploring for a test suite.

One suggestion is trying to avoid I/O and instead making tests work in-memory. The cargo test suite seems to use builders for setting up "multi-file" tests, see e.g. https://github.com/rust-lang/cargo/blob/master/tests/testsuite/build_plan.rs. UFOs are multi-file objects after all. The builder can take strings for input to exercise parsers. The only thing you'd cut out would be read calls to the filesystem.

madig commented 3 years ago

This may also work for projects like ufofmt @chrissimpkins

cmyr commented 3 years ago

Is this addressing concerns with the runtime of our testing?

madig commented 3 years ago

The ideas touch runtime performance and test maintainability. Not saying we have issues with either, but the ideas strike me as desirable.

cmyr commented 3 years ago

Totally, it's important for tests to be fast and reasonably easy to maintain. That said, given norad's modest scope I wouldn't worry too much about the performance aspect until an actual problem arises, which I would not expect to really happen ever?

For maintainability, I do think that the general practice of preferring integration tests over lots of unit tests makes sense, and I think this is what a lot of our tests are doing: for instance when we have a test that dumps some file to xml and then compare that to a known-good result, it is essentially an integration test.

madig commented 3 years ago

Certainly, but we currently have the additional layer of external files.

I want to have a closer look at the test suite at some point and extend/prune it; I may experiment a bit then.

cmyr commented 3 years ago

sounds good!

One eventual thing we might want to do is move the test data into a subdirectory.

madig commented 3 years ago

I just saw that rust-analyzer is also running other stuff in tests: https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/rust-analyzer/tests/slow-tests/tidy.rs

madig commented 3 years ago

This might be useful: https://docs.rs/expect-test/1.1.0/expect_test/

chrissimpkins commented 3 years ago

This may also work for projects like ufofmt @chrissimpkins

Will give it a read. Ty Niko!