Conversation
6e09dde to
ae3e4b3
Compare
|
|
||
| use super::rustfluent; | ||
|
|
||
| #[test] |
There was a problem hiding this comment.
If you look into using rstest it has a slightly nicer handling of parametrisation than vanilla rust: https://rstest.rs/
We've used it in a few other areas in Flex
There was a problem hiding this comment.
Interesting tool. Never looked into it. I might try it out sometime and introduce it in a separate PR.
ae3e4b3 to
12c485e
Compare
seddonym
left a comment
There was a problem hiding this comment.
A bit on the fence about this change. On the one hand, it's nice to have a slightly faster feedback loop for running tests, and maybe a better experience if we want to drop into an interactive debugger.
On the other hand, since this is a Python API, I think the Python tests should be where we concentrate the high level test coverage. Having Rust and Python tests can make it harder to know where to put a test, and increases the risk of inadequate coverage in the Python tests.
In other libraries I maintain, my general approach is that it should be ok to delete all the Rust tests at any point, they're just there to help with implementation.
Another reason for not doing this is that when we make a change, we will have to change the tests in too places, which may slow devs down.
I don't feel too strongly, but those are the counterarguments. If you genuinely feel this is necessary then feel free to merge it.
|
I though a little bit about Davids comment regarding the necessity of having dedicated Rust unit tests in parallel to the python tests. I will drop the PR for now in order to favor only having Python tests for now. |
In order to prepare for some upcoming changes, I added some basic tests in order to be able test changes in Rust locally.