Add tests documenting date range formatter fallback issues - #8360
Conversation
d8ea61e to
6eed6f9
Compare
| let res_zh = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { | ||
| fmt_zh.format(&start, &end_day).to_string() | ||
| })); | ||
| assert!( | ||
| res_zh.is_err(), | ||
| "Expected error due to NamesNotLoaded when zh range falls back to root pattern with MMM" | ||
| ); |
There was a problem hiding this comment.
Issue: I think we probably shouldn't land std::panic::catch_unwind. Also, note that the panic is a debug_assert in impl Writeable.
Suggestion: avoid std::panic::catch_unwind by using a power-user API. I think there are 2 ways in the single formatter code path:
- Use
format_unchecked -> FormattedDateTimeUnchecked, which returns aTryWriteable, which you can inspect for an error - Use
format -> FormattedDateTime -> patternandDateTimeNames::from_formatterto manually format the pattern
This might need new APIs for DateTimeRangeFormatter, but those are APIs we would want anyway for graduation.
There was a problem hiding this comment.
format_unchecked seems like a good call
There was a problem hiding this comment.
We don't have format_unchecked, are you saying I should add it? I'd rather not go that far.
There was a problem hiding this comment.
The thing to do here is instead to use should_panic and only run it in debug mode
There was a problem hiding this comment.
We don't have
format_unchecked, are you saying I should add it? I'd rather not go that far.
I said in my first comment,
This might need new APIs for DateTimeRangeFormatter, but those are APIs we would want anyway for graduation.
but I'm okay with should_panic.
6eed6f9 to
5a6bc85
Compare
|
dart CI: |
Co-authored-by: Shane F. Carr <shane@unicode.org>
#8359 documents an issue with
unddata fallback in range formattingThis shows that we have that issue, and which locales we have it in, so that the work to fix it has tests to update.
(It's more convincing when you have currently-failing tests to fix, rather than when you show some passing tests and say "trust me these cases were broken before")
Changelog (N/A)