Skip to content

Add tests documenting date range formatter fallback issues - #8360

Merged
Manishearth merged 2 commits into
unicode-org:mainfrom
Manishearth:range-fallback-tests
Aug 13, 2026
Merged

Add tests documenting date range formatter fallback issues#8360
Manishearth merged 2 commits into
unicode-org:mainfrom
Manishearth:range-fallback-tests

Conversation

@Manishearth

@Manishearth Manishearth commented Aug 12, 2026

Copy link
Copy Markdown
Member

#8359 documents an issue with und data fallback in range formatting

This 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)

@Manishearth Manishearth changed the title agent-driven: Add tests documenting date range formatter fallback issues (#8359) Add tests documenting date range formatter fallback issues Aug 12, 2026
@Manishearth
Manishearth force-pushed the range-fallback-tests branch from d8ea61e to 6eed6f9 Compare August 12, 2026 20:53
Comment thread components/datetime/tests/range.rs
Comment thread components/datetime/tests/range.rs Outdated
Comment thread components/datetime/tests/range.rs Outdated
Comment on lines +415 to +421
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"
);

@sffc sffc Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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:

  1. Use format_unchecked -> FormattedDateTimeUnchecked, which returns a TryWriteable, which you can inspect for an error
  2. Use format -> FormattedDateTime -> pattern and DateTimeNames::from_formatter to manually format the pattern

This might need new APIs for DateTimeRangeFormatter, but those are APIs we would want anyway for graduation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

format_unchecked seems like a good call

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't have format_unchecked, are you saying I should add it? I'd rather not go that far.

@Manishearth Manishearth Aug 12, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The thing to do here is instead to use should_panic and only run it in debug mode

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Made an attempt.

@sffc sffc Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@Manishearth
Manishearth force-pushed the range-fallback-tests branch from 6eed6f9 to 5a6bc85 Compare August 12, 2026 21:14
@Manishearth
Manishearth requested a review from sffc August 12, 2026 21:18
@sffc

sffc commented Aug 12, 2026

Copy link
Copy Markdown
Member

dart CI:

yq v4.53.3 [Approved]
yq package files install completed. Performing other installation steps.
Attempt to get headers for https://github.com/mikefarah/yq/releases/download/v4.53.3/yq_windows_amd64.exe failed.
  The remote file either doesn't exist, is unauthorized, or is forbidden for url 'https://github.com/mikefarah/yq/releases/download/v4.53.3/yq_windows_amd64.exe'. Exception calling "GetResponse" with "0" argument(s): "The underlying connection was closed: The connection was closed unexpectedly."
Downloading yq 64 bit
  from 'https://github.com/mikefarah/yq/releases/download/v4.53.3/yq_windows_amd64.exe'
ERROR: The remote file either doesn't exist, is unauthorized, or is forbidden for url 'https://github.com/mikefarah/yq/releases/download/v4.53.3/yq_windows_amd64.exe'. Exception calling "GetResponse" with "0" argument(s): "The underlying connection was closed: The connection was closed unexpectedly." 
This package is likely not broken for licensed users - see https://docs.chocolatey.org/en-us/features/private-cdn.
The install of yq was NOT successful.
Error while running 'C:\ProgramData\chocolatey\lib\yq\tools\chocolateyinstall.ps1'.
 See log for details.

Chocolatey installed 0/1 packages. 1 packages failed.
 See the log for details (C:\ProgramData\chocolatey\logs\chocolatey.log).

Failures
 - yq (exited 404) - Error while running 'C:\ProgramData\chocolatey\lib\yq\tools\chocolateyinstall.ps1'.
 See log for details.

Comment thread components/datetime/tests/range.rs Outdated
Co-authored-by: Shane F. Carr <shane@unicode.org>
@Manishearth
Manishearth enabled auto-merge (squash) August 12, 2026 22:15
@Manishearth
Manishearth requested a review from sffc August 12, 2026 22:15
@Manishearth
Manishearth merged commit 71fd1f4 into unicode-org:main Aug 13, 2026
34 checks passed
@Manishearth
Manishearth deleted the range-fallback-tests branch August 13, 2026 13:30
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.

2 participants