Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix failing QueryParser Flexible tests due to zh-Hant-TW culture bug, #846 #1078

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

paulirwin
Copy link
Contributor

@paulirwin paulirwin commented Dec 31, 2024

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Fixes tests that fail randomly when the randomly selected culture is zh-Hant-TW and the time zone offset is negative.

Partial #846

Description

See this comment for full details explaining the problem: #846 (comment)

The solution was to ensure that the zh-Hant-TW culture (on .NET 6-8 when it is missing the tt in the LongTimePattern) does not pass the "sanity" check when there is a time zone with a negative offset due to this round-trip formatting/parsing bug. There already was a sanity check for the unix epoch (long value 0) which ensured the round-trip to/from string was parsed successfully, but it was not checking that it was parsed correctly. This code had been modified heavily from the original Lucene and was missing this equality check.

While investigating this, there were opportunities to clean up the code in this class, as well as to make it support using the [Repeat] attribute to help in diagnosing issues. The use of String was normalized to the string keyword, some redundant parentheses were removed, and the method capitalization was fixed. To enable [Repeat] support, several fields were moved from static to instance fields, and OneTimeSetUp/OneTimeTearDown was replaced with SetUp/TearDown. This allows for generating new random values when you repeat the test. Additionally, some extra info was added to the test assertion message to aid diagnosing issues if it fails.

@paulirwin paulirwin added the notes:bug-fix Contains a fix for a bug label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes:bug-fix Contains a fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant