Skip to content

Add some basic rust unit tests#36

Closed
jilsahm wants to merge 1 commit intomainfrom
refactor/enable-rust-tests
Closed

Add some basic rust unit tests#36
jilsahm wants to merge 1 commit intomainfrom
refactor/enable-rust-tests

Conversation

@jilsahm
Copy link

@jilsahm jilsahm commented Jan 27, 2026

In order to prepare for some upcoming changes, I added some basic tests in order to be able test changes in Rust locally.

@jilsahm jilsahm force-pushed the refactor/enable-rust-tests branch from 6e09dde to ae3e4b3 Compare January 27, 2026 12:59

use super::rustfluent;

#[test]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting tool. Never looked into it. I might try it out sometime and introduce it in a separate PR.

@jilsahm jilsahm force-pushed the refactor/enable-rust-tests branch from ae3e4b3 to 12c485e Compare January 27, 2026 14:35
@jilsahm jilsahm marked this pull request as ready for review January 27, 2026 15:01
@jilsahm jilsahm requested a review from a team as a code owner January 27, 2026 15:01
Copy link
Collaborator

@seddonym seddonym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@jilsahm
Copy link
Author

jilsahm commented Jan 30, 2026

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.

@jilsahm jilsahm closed this Jan 30, 2026
@LilyFirefly LilyFirefly deleted the refactor/enable-rust-tests branch January 30, 2026 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants