search

exercism / rust

Exercism exercises in Rust.
https://exercism.org/tracks/rust
MIT License
1.44k stars 520 forks source link

Add test that templates match generated test suite #1878

Closed senekor closed 5 months ago

senekor commented 5 months ago

Every exercise that already has a .meta/test_template.tera file should be tested in CI that the template actually matches the test file on disk. This should help with adding new exercises so people don't forget to use the template and write the test suite manually. Also, it will help when refactoring the exercise generator, to prevent mistakes that break test generation for some exercise. With such a test, that would be noticed only when the affected exercise would be updated the next time.

jpal91 commented 5 months ago

Since I know this one is my fault, I'm willing to help out! Haha.

Should be fairly straight-forward, though, from what I see. Most all of the code you would need is already in rust-tooling/generate so you could just read the file to string and see if it equals the result of generate_tests(). generate would just need to be added as a dependency to ci-tests.

Let me know!

jpal91 commented 5 months ago

Actually I take that last part back now that I look at it more. No sense in compiling the entire crate just for 1-2 function(s). Copy + paste would be better but if anything changes with generate_tests() or filters get added down the line (like mentioned in CONTRIBUTING.md), that would cause an issue.

Maybe we could move those functions to rust-tooling/utils instead?

senekor commented 5 months ago

Maybe that was the reason I didn't add a test like that in the first place when I wrote the generator. However we slice and dice the crates, test generation is always going to compile tera, which does take a while.

A few considerations that come to mind right now:

With regards to you taking over the work, I'm not sure you'd have a great experience. There's a good chance that a bunch of the test templates are actually broken right now. The problem is that I'm not completely happy with how the Rust generator interacts with tera templates and json from upstream... one part of the chain always ends up more complicated and ugly than I would like. A good solution here might involve changing how the generator works to some extent and refactoring many existing tera templates.

Apologies, I opened the issue as a sort of note to myself, it was not yet thought through 100%.

jpal91 commented 5 months ago

That makes sense. It does look a little tricky from the code base.

Maybe do something half-way until you have to push to main? Like pull the function names, and check that they are all present. There will still have to be a more concrete test that compares the parsed tera later, but at least you'd avoid more overhead on most PRs.

I may look around a little bit more and see if I notice anything else, though. I agree it's definitely something that needs to be added.

senekor commented 5 months ago

I've added a test case in #1879. It doesn't run in CI at all, but should come in handy to keep everything tidy manually. (It discovered three or four oversights already.)

I'd say it doesn't make sense to try to push it further before we approach the long-term goal of https://github.com/exercism/rust/issues/1721.