Additional tests for FlexboxLayout and FlexDirection#174
Additional tests for FlexboxLayout and FlexDirection#174alice-i-cecile merged 6 commits intoDioxusLabs:mainfrom
Conversation
alice-i-cecile
left a comment
alice-i-cecile
left a comment
There was a problem hiding this comment.
Everything else being equal, I'd rather use multiple asserts in a single test than the rstest macro.
It will be faster to build, and easier to refactor. For example I don't think that the flex_direction_is_row tests are any clearer or shorter using this syntax than with four asserts.
src/style.rs
Outdated
| #[rstest] | ||
| #[case( | ||
| FlexDirection::Row, | ||
| Rect { start: Dimension::Points(1.0), top: Dimension::Points(2.0), ..Default::default() }, |
There was a problem hiding this comment.
Can we use methods in these invocations? Because if so, we should consider creating some convenience methods to make using and testing these structs clearer.
There was a problem hiding this comment.
We can! I'll create some.
|
Definitely like the direction we're moving in here; unit tests for the individual steps will make refactors much easier and because we're building to a spec these tests should be very stable. |
|
So I've ended up removing all rtest macros, leaving only simple asserts. Also created some helpers functions as you suggested, it did clean up a lot. Let me know if there's something else to improve! |
src/style.rs
Outdated
| mod test_flexbox_layout { | ||
| use crate::style::*; | ||
|
|
||
| fn size_from_points(width: f32, height: f32) -> Size<Dimension> { |
There was a problem hiding this comment.
I kind of want these to be methods on Size / Rect / FlexboxLayout. Thoughts?
There was a problem hiding this comment.
Yeah I think that makes sense :)
There was a problem hiding this comment.
I think that it works great for the Size and Rect, they're nice shortcurts, but I'm not sure if it's useful for FlexboxLayout.
It seems a bit arbitrary to have only from_align_self and from_align_items and even then, I'm not sure if it's a common use case. Would love to have some shorthand for initializing a default FlexboxLayout though.
So I would prefer any of those two options for FlexboxLayout:
- Implement
from_align_self,from_align_itemsand every otherfrom_x - Don't implement any of them (I'm partial to this)
What do you guys think?
There was a problem hiding this comment.
I vote for adding these convenience methods to Size and Rect, then we can skip any changes to FlexboxLayout for now then revisit later.
There was a problem hiding this comment.
Great! I've added them and updated the tests. Let me know if there's anything I can improve.
|
This is looking much better. CI failure isn't your fault (nightly got new lints); feel free to open a new PR applying the suggested fixes and I can merge that in first. |
|
I'll merge this once those methods are moved :) |
* Additional tests for FlexboxLayout and FlexDirection * Refactoring FlexboxLayout and FlexDirection tests * Adding helper functions to Size and Rect Co-authored-by: Marcello Vaz <[email protected]> Co-authored-by: Alice Cecile <[email protected]>
4 participants
Objective
Continuing work towards #92.
Feedback wanted
The tests ended up being a little repetitive, although still simple. Would love some suggestions on how to make them more succinct.