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

Task: Investigate failing QueryParser Flexible tests (TestNumericQueryParser.cs) #846

Open
NightOwl888 opened this issue Apr 28, 2023 · 9 comments

Comments

@NightOwl888
Copy link
Contributor

This appears to be a rare edge case, as it hasn't come up before. But then, we have only recently added tests for .NET 7.0. These tests failed on ubuntu-latest (Ubuntu 22.04) using net7.0. Since they failed as a group, they likely have the same underlying cause.

https://dev.azure.com/lucene-net-temp4/Main/_build/results?buildId=232&view=ms.vss-test-web.build-test-results-tab

Note the above link will be cleaned up within 1 month, so the details have been copied below.

TestInclusiveLowerNumericRange

Details

Click to expand

Error Message

Expected: 1, Actual: 0

Query <+INT32:{"-608201030" TO "0"] +INT64:{"-9004819226736869376" TO "0"] +SINGLE:{"-0.49551165" TO "0"] +DOUBLE:{"-0.4581616751646118" TO "0"] +date:{"1908/9/28 5:02:10 -6 西元 10 -06:00 1908" TO "1969/12/31 7:00:00 -5 西元 0 -05:00 1969"]> retrieved 0 document(s), 1 document(s) expected.

To reproduce this test result:

Option 1:

Apply the following assembly-level attributes:

[assembly: Lucene.Net.Util.RandomSeed("0x1aea3b735c59df90")]
[assembly: NUnit.Framework.SetCulture("ff-Latn-GM")]

Option 2:

Use the following .runsettings file:

<RunSettings>
<TestRunParameters>
<Parameter name="tests:seed" value="0x1aea3b735c59df90" />
<Parameter name="tests:culture" value="ff-Latn-GM" />
</TestRunParameters>
</RunSettings>

See the .runsettings documentation at: https://docs.microsoft.com/en-us/visualstudio/test/configure-unit-tests-by-using-a-dot-runsettings-file.

Stack Trace

at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestQuery(String queryStr, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 651
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.assertRangeQuery(Nullable1 lowerType, Nullable1 upperType, Boolean lowerInclusive, Boolean upperInclusive, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 568
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestInclusiveLowerNumericRange() in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 427

TestOpenRangeNumericQuery

Details

Click to expand

Error Message

Expected: 2, Actual: 1

Query <+INT32<="0" +INT64<="0" +SINGLE<="0" +DOUBLE<="0" +date<="1969/12/31 7:00:00 -5 西元 0 -05:00 1969"> retrieved 1 document(s), 2 document(s) expected.

To reproduce this test result:

Option 1:

Apply the following assembly-level attributes:

[assembly: Lucene.Net.Util.RandomSeed("0x1aea3b735c59df90")]
[assembly: NUnit.Framework.SetCulture("ff-Latn-GM")]

Option 2:

Use the following .runsettings file:

<RunSettings>
<TestRunParameters>
<Parameter name="tests:seed" value="0x1aea3b735c59df90" />
<Parameter name="tests:culture" value="ff-Latn-GM" />
</TestRunParameters>
</RunSettings>

See the .runsettings documentation at: https://docs.microsoft.com/en-us/visualstudio/test/configure-unit-tests-by-using-a-dot-runsettings-file.

Stack Trace

at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestQuery(String queryStr, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 651
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.assertOpenRangeQuery(NumberType boundType, String operator, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 601
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestOpenRangeNumericQuery() in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 461

TestInclusiveNumericRange

Details

Click to expand

Error Message

Expected: 1, Actual: 0

Query <+INT32:["0" TO "0"] +INT64:["0" TO "0"] +SINGLE:["0" TO "0"] +DOUBLE:["0" TO "0"] +date:["1969/12/31 7:00:00 -5 西元 0 -05:00 1969" TO "1969/12/31 7:00:00 -5 西元 0 -05:00 1969"]> retrieved 0 document(s), 1 document(s) expected.

To reproduce this test result:

Option 1:

Apply the following assembly-level attributes:

[assembly: Lucene.Net.Util.RandomSeed("0x1aea3b735c59df90")]
[assembly: NUnit.Framework.SetCulture("ff-Latn-GM")]

Option 2:

Use the following .runsettings file:

<RunSettings>
<TestRunParameters>
<Parameter name="tests:seed" value="0x1aea3b735c59df90" />
<Parameter name="tests:culture" value="ff-Latn-GM" />
</TestRunParameters>
</RunSettings>

See the .runsettings documentation at: https://docs.microsoft.com/en-us/visualstudio/test/configure-unit-tests-by-using-a-dot-runsettings-file.

Stack Trace

at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestQuery(String queryStr, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 651
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.assertRangeQuery(Nullable1 lowerType, Nullable1 upperType, Boolean lowerInclusive, Boolean upperInclusive, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 568
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestInclusiveNumericRange() in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 415

TestSimpleNumericQuery

Details

Click to expand

Error Message

Expected: 1, Actual: 0

Query <+INT32:"0" +INT64:"0" +SINGLE:"0" +DOUBLE:"0" +date:"1969/12/31 7:00:00 -5 西元 0 -05:00 1969"> retrieved 0 document(s), 1 document(s) expected.

To reproduce this test result:

Option 1:

Apply the following assembly-level attributes:

[assembly: Lucene.Net.Util.RandomSeed("0x1aea3b735c59df90")]
[assembly: NUnit.Framework.SetCulture("ff-Latn-GM")]

Option 2:

Use the following .runsettings file:

<RunSettings>
<TestRunParameters>
<Parameter name="tests:seed" value="0x1aea3b735c59df90" />
<Parameter name="tests:culture" value="ff-Latn-GM" />
</TestRunParameters>
</RunSettings>

See the .runsettings documentation at: https://docs.microsoft.com/en-us/visualstudio/test/configure-unit-tests-by-using-a-dot-runsettings-file.

Stack Trace

at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestQuery(String queryStr, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 651
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.assertSimpleQuery(NumberType numberType, Int32 expectedDocCount) in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 633
at Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestSimpleNumericQuery() in /_/src/Lucene.Net.Tests.QueryParser/Flexible/Standard/TestNumericQueryParser.cs:line 495

@NightOwl888 NightOwl888 added up-for-grabs This issue is open to be worked on by anyone help-wanted Extra attention is needed is:bug test-failure pri:normal labels Apr 28, 2023
@NightOwl888
Copy link
Contributor Author

@paulirwin - I got another failure on this today on macOS and net7.0. Since you have a mac, think you can take a look?

https://dev.azure.com/lucene-net-temp4/Main/_build/results?buildId=267&view=ms.vss-test-web.build-test-results-tab

[assembly: Lucene.Net.Util.RandomSeed("0xfcd3b1a5ebdbe7c0")]
[assembly: NUnit.Framework.SetCulture("ks-Arab-IN")]

I don't think this has ever failed on Windows. It possibly has something to do with either the recent time zone changes to DateTools (#853) or the upgrade of the TimeZoneConverter NuGet package that we use in tests.

@paulirwin
Copy link
Contributor

@NightOwl888 Unfortunately for our diagnostics, these tests passed on my Mac with that seed/culture set and net7.0:

i.e. TestInclusiveLowerNumericRange:

RandomSeed: 0xfcd3b1a5ebdbe7c0
Culture: ks-Arab-IN
Time Zone: (UTC+12:00) Marshall Islands Time
Default Codec: Lucene40 (Lucene40RWCodec)
Default Similarity: DefaultSimilarity
Nightly: False
Weekly: False
Slow: True
Awaits Fix: False
Directory: random
Verbose: True
Random Multiplier: 1
Parsing: +INT32:{"-2096763321" TO "0"] +INT64:{"-1081678821072489728" TO "0"] +SINGLE:{"-0,046893597" TO "0"] +DOUBLE:{"-0,05523188455414629" TO "0"] +date:{"1932 de julhet 3 17:06:43 CE 43 +07:00 1932" TO "1970 de genièr 1 07:00:00 CE 0 +07:00 1970"]
Querying: +INT32:{‎-‎2096763321 TO 0] +INT64:{‎-‎1081678821072489728 TO 0] +SINGLE:{‎-‎0٫046893597 TO 0] +DOUBLE:{‎-‎0٫05523188455414629 TO 0] +date:{‎-‎1183297997000 TO 0]
Query <+INT32:{"-2096763321" TO "0"] +INT64:{"-1081678821072489728" TO "0"] +SINGLE:{"-0,046893597" TO "0"] +DOUBLE:{"-0,05523188455414629" TO "0"] +date:{"1932 de julhet 3 17:06:43 CE 43 +07:00 1932" TO "1970 de genièr 1 07:00:00 CE 0 +07:00 1970"]> retrieved 1 document(s), 1 document(s) expected.
Parsing: +INT32:{"0" TO "2096763321"] +INT64:{"0" TO "1081678821072489728"] +SINGLE:{"0" TO "0,046893597"] +DOUBLE:{"0" TO "0,05523188455414629"] +date:{"1970 de genièr 1 07:00:00 CE 0 +07:00 1970" TO "2007 de julhet 1 22:53:17 CE 17 +09:00 2007"]
Querying: +INT32:{0 TO 2096763321] +INT64:{0 TO 1081678821072489728] +SINGLE:{0 TO 0٫046893597] +DOUBLE:{0 TO 0٫05523188455414629] +date:{0 TO 1183297997000]
Query <+INT32:{"0" TO "2096763321"] +INT64:{"0" TO "1081678821072489728"] +SINGLE:{"0" TO "0,046893597"] +DOUBLE:{"0" TO "0,05523188455414629"] +date:{"1970 de genièr 1 07:00:00 CE 0 +07:00 1970" TO "2007 de julhet 1 22:53:17 CE 17 +09:00 2007"]> retrieved 1 document(s), 1 document(s) expected.
Parsing: +INT32:{"-2096763321" TO "2096763321"] +INT64:{"-1081678821072489728" TO "1081678821072489728"] +SINGLE:{"-0,046893597" TO "0,046893597"] +DOUBLE:{"-0,05523188455414629" TO "0,05523188455414629"] +date:{"1932 de julhet 3 17:06:43 CE 43 +07:00 1932" TO "2007 de julhet 1 22:53:17 CE 17 +09:00 2007"]
Querying: +INT32:{‎-‎2096763321 TO 2096763321] +INT64:{‎-‎1081678821072489728 TO 1081678821072489728] +SINGLE:{‎-‎0٫046893597 TO 0٫046893597] +DOUBLE:{‎-‎0٫05523188455414629 TO 0٫05523188455414629] +date:{‎-‎1183297997000 TO 1183297997000]
Query <+INT32:{"-2096763321" TO "2096763321"] +INT64:{"-1081678821072489728" TO "1081678821072489728"] +SINGLE:{"-0,046893597" TO "0,046893597"] +DOUBLE:{"-0,05523188455414629" TO "0,05523188455414629"] +date:{"1932 de julhet 3 17:06:43 CE 43 +07:00 1932" TO "2007 de julhet 1 22:53:17 CE 17 +09:00 2007"]> retrieved 2 document(s), 2 document(s) expected.
Parsing: +INT32:{"-2096763321" TO "-2096763321"] +INT64:{"-1081678821072489728" TO "-1081678821072489728"] +SINGLE:{"-0,046893597" TO "-0,046893597"] +DOUBLE:{"-0,05523188455414629" TO "-0,05523188455414629"] +date:{"1932 de julhet 3 17:06:43 CE 43 +07:00 1932" TO "1932 de julhet 3 17:06:43 CE 43 +07:00 1932"]
Querying: +INT32:{‎-‎2096763321 TO ‎-‎2096763321] +INT64:{‎-‎1081678821072489728 TO ‎-‎1081678821072489728] +SINGLE:{‎-‎0٫046893597 TO ‎-‎0٫046893597] +DOUBLE:{‎-‎0٫05523188455414629 TO ‎-‎0٫05523188455414629] +date:{‎-‎1183297997000 TO ‎-‎1183297997000]
Query <+INT32:{"-2096763321" TO "-2096763321"] +INT64:{"-1081678821072489728" TO "-1081678821072489728"] +SINGLE:{"-0,046893597" TO "-0,046893597"] +DOUBLE:{"-0,05523188455414629" TO "-0,05523188455414629"] +date:{"1932 de julhet 3 17:06:43 CE 43 +07:00 1932" TO "1932 de julhet 3 17:06:43 CE 43 +07:00 1932"]> retrieved 0 document(s), 0 document(s) expected.

As an aside: the documented lucene.testSettings.config approach doesn't seem to work at least as documented on my Mac. I haven't yet dug into why, but I created that file in the repo root (which should be "between the executable and the root of the drive") and it didn't take the culture/seed. Instead I added those attributes as you provided to the QueryParser tests AssemblyInfo.cs directly. Let me know if there's a better way to do that. Thanks!

@NightOwl888
Copy link
Contributor Author

Yeah, that is what happened when I tried chasing this down on Ubuntu. It seems that this is due to the exact system configuration that the test is running in. It may be easier just to revert those 2 time zone changes one at a time to see if the error stops happening, although it takes lots of runs to get a failure.

lucene.testSettings.config - maybe try the name all lowercase?

Instead I added those attributes as you provided to the QueryParser tests AssemblyInfo.cs directly. Let me know if there's a better way to do that. Thanks!

The attribute is what I usually use because it overrides any other config on the system.

It is possible to specify system properties in a .runsettings file.

Use the following .runsettings file:


<RunSettings>
  <TestRunParameters>
    <Parameter name="tests:seed" value="0xfcd3b1a5ebdbe7c0" />
    <Parameter name="tests:culture" value="ks-Arab-IN" />
  </TestRunParameters>
</RunSettings>
See the .runsettings documentation at: https://docs.microsoft.com/en-us/visualstudio/test/configure-unit-tests-by-using-a-dot-runsettings-file.

It is also possible to pass them via command line, but the syntax is pretty weird and varies by shell: https://github.com/Microsoft/vstest-docs/blob/main/docs/RunSettingsArguments.md. There is an example of this being done in TestIndexWriterOnJRECrash.

@paulirwin paulirwin added this to the 4.8.0-beta00018 milestone Oct 28, 2024
@NightOwl888
Copy link
Contributor Author

I got another failure. This time on Linux: https://dev.azure.com/shad0962/Experiments/_build/results?buildId=2528&view=ms.vss-test-web.build-test-results-tab&runId=1006431&resultId=100248&paneView=debug.

Reposing the repeat data here, since the CI environment will eventually be cleaned up.

Expected: 1, Actual: 0


Query <+INT32:{"0" TO "554807661"] +INT64:{"0" TO "4750245968736588800"] +SINGLE:{"0" TO "0.55612606"] +DOUBLE:{"0" TO "0.27287609303204485"] +date:{"1970年1月1日 星期四 4:00 西元 0 +04:00 1970" TO "1993年9月30日 星期四 9:15 西元 14 +04:00 1993"]> retrieved 0 document(s), 1 document(s) expected.


To reproduce this test result:


Option 1:


Apply the following assembly-level attributes:


[assembly: Lucene.Net.Util.RandomSeed("0xb19ea259d1e18b66")]
[assembly: NUnit.Framework.SetCulture("es-BZ")]


Option 2:


Use the following .runsettings file:


<RunSettings>
  <TestRunParameters>
    <Parameter name="tests:seed" value="0xb19ea259d1e18b66" />
    <Parameter name="tests:culture" value="es-BZ" />
  </TestRunParameters>
</RunSettings>
Option 3:


Create the following lucene.testsettings.json file somewhere between the test assembly and the root of your drive:


{
"tests": {
"seed": "0xb19ea259d1e18b66",
"culture": "es-BZ"
}
}

@paulirwin paulirwin self-assigned this Dec 26, 2024
@paulirwin
Copy link
Contributor

While investigating this, I needed to repeat these tests but with a new test/seed each time, and they use static values with OneTimeSetUp so I needed to repeat the entire dotnet test run. (Aside: I also temporarily, locally made these not-OneTime/static for adding a Repeat attribute as well, but still use the script below.) I whipped up this little Powershell script to re-run a specific test, without running build, over and over again until it fails. Hope this helps someone (or future me):

do { 
    dotnet test ./src/Lucene.Net.Tests.QueryParser/Lucene.Net.Tests.QueryParser.csproj -c Release --filter "FullyQualifiedName=Lucene.Net.QueryParsers.Flexible.Standard.TestNumericQueryParser.TestInclusiveNumericRange" --no-build 
} until (!$?)

Make sure to remove any fixed seed/culture in your test settings first (unless you're trying to track down a concurrency bug).

@paulirwin
Copy link
Contributor

After many, many attempts, finally was able to reproduce a failure on Linux x64:

Query <+INT32:["0" TO "2104943488"] +INT64:["0" TO "8987044613491660800"] +SINGLE:["0" TO "0.074273415"] +DOUBLE:["0" TO "0.8280274472269546"] +date:["1970/1/1 4:00:00 +4 西元 0 +04:00 1970" TO "2004/11/12 1:42:19 +3 西元 19 +03:00 2004"]> retrieved 1 document(s), 2 document(s) expected.

test settings:

{
  "tests": {
     "seed": "0xc700d5224e7f9ffc",
     "culture": "ff-Latn-SL"
  }
}

The common thread seems to be cultures with "西元" as the era name (akin to "AD" in en-US). These cultures on my Linux machine are:

  • yue
  • yue-Hans
  • yue-Hans-CN
  • yue-Hant
  • yue-Hant-HK
  • zh-Hant
  • zh-Hant-TW

macOS also has these additional cultures with that era name:

  • zh-Hant-CN
  • zh-Hant-JP

Note that the culture in the test settings is effectively ignored, a random culture is picked for these tests based on the random seed. Also this cannot be reproduced on macOS with the same seed, likely because the list of cultures is different.

paulirwin added a commit to paulirwin/lucene.net that referenced this issue Dec 30, 2024
paulirwin added a commit to paulirwin/lucene.net that referenced this issue Dec 30, 2024
paulirwin added a commit to paulirwin/lucene.net that referenced this issue Dec 31, 2024
@paulirwin
Copy link
Contributor

I am happy to report that I have definitively found the source of the failures from this original issue. It occurs only for the zh-Hant-TW culture, for any time zones with a negative offset, and only on .NET 6-8 on Linux and macOS.

The root cause is that that culture's CultureInfo is missing the tt AM/PM designator from the format strings in DateTimeFormat.LongTimePattern (as well as FullDateTimePattern, but we aren't using that). So the following code:

using System.Globalization;
var ci = CultureInfo.GetCultureInfo("zh-Hant-TW");
Console.WriteLine(ci.DateTimeFormat.LongDatePattern + " " + ci.DateTimeFormat.LongTimePattern);
Console.WriteLine(new DateTime(1969, 12, 31, 20, 0, 0).ToString(ci.DateTimeFormat.LongDatePattern + " " + ci.DateTimeFormat.LongTimePattern, ci));

... results in the following on .NET 6-8 on Linux and macOS:

yyyy年M月d日 dddd h:mm:ss
1969年12月31日 星期三 8:00:00

... whereas it results in the following on .NET 9, because it includes the tt format before the hour:

yyyy年M月d日 dddd tth:mm:ss
1969年12月31日 星期三 下午8:00:00

Without the 下午, this gets parsed as 8:00 am on the day before the unix epoch (in this case, at a -4:00 offset time zone), rather than 8:00 pm, which when adjusted for the timezone offset is the unix epoch exactly. Because the time is 12 hours before the epoch, the documents do not match the date queries, and the expected number of results is not returned, thus the test assertion fails. This is only a problem with negative offsets, because with zero or positive offsets, it is a number on or after midnight which will correctly get parsed as AM without the designator.

I wrote a small program to go through and verify all cultures to see if any others were a problem like this, and it seems to only be zh-Hant-TW, and only net6.0-net8.0. The .NET team seems to have fixed this (possibly unintentionally by upgrading ICU) in .NET 9.

I am going to fix this by adding another form of "sanity" check for the culture/time zone combinations that ensures that the unix epoch can round-trip ToString/Parse with the given format string. If it fails, then it'll iterate again and find a new random culture/time zone that works.

Additionally, I found another failure through many repeated random runs, that had not been reported yet. For cultures that use a decimal comma, such as sv-FI, small decimal values can fail due to a J2N round-trip formatting/parsing bug when there is a decimal comma and exponential notation. That has been filed as NightOwl888/J2N#128.

@paulirwin
Copy link
Contributor

Update: it fails on Windows on net6.0-net8.0 as well, and is fixed on net9.0 there too. I had just missed this because I had only previously tested that on net9.0, and had not yet made the connection to this being a .NET version-specific bug. (And, it fails on net5.0 as well, but did not test those earlier since we no longer have that test framework target.)

@paulirwin
Copy link
Contributor

Status update. To recap, there are two issues that have caused failing QueryParser Flexible tests here. Fix PRs are available for each:

  1. The zh-Hant-TW issue mentioned in detail above, fixed by Fix failing QueryParser Flexible tests due to zh-Hant-TW culture bug, #846 #1078
  2. The J2N parsing round-trip issue, fixed by Fix issue with non-hyphen negative signs in exp notation, #128 NightOwl888/J2N#129

The latter will require an update to the J2N dependency when an updated version with that fix is available. We'll consider this issue fixed when both of these are merged/updated.

@paulirwin paulirwin added awaits:dependency and removed up-for-grabs This issue is open to be worked on by anyone help-wanted Extra attention is needed labels Dec 31, 2024
paulirwin added a commit that referenced this issue Jan 6, 2025
…846 (#1078)

* Clean up TestNumericQueryParser code style and make static methods instance, #846

* Add extra time zone logging, #846

* Fixes issue with zh-Hant-TW culture and negative offset time zones, #846
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants