NightOwl888 commented on code in PR #1054:
URL: https://github.com/apache/lucenenet/pull/1054#discussion_r1867386446
##########
src/Lucene.Net.Tests/Search/TestMultiTermQueryRewrites.cs:
##########
@@ -243,36 +241,44 @@ public virtual void TestBoosts()
CheckBoosts(new
MultiTermQuery.TopTermsScoringBooleanQueryRewrite(1024));
}
- private void CheckMaxClauseLimitation(MultiTermQuery.RewriteMethod
method, [CallerMemberName] string memberName = "")
+ // LUCENENET specific - made static
+ private static void
CheckMaxClauseLimitation(MultiTermQuery.RewriteMethod method)
{
int savedMaxClauseCount = BooleanQuery.MaxClauseCount;
BooleanQuery.MaxClauseCount = 3;
MultiTermQuery mtq = TermRangeQuery.NewStringRange("data", "2",
"7", true, true);
- mtq.MultiTermRewriteMethod = (method);
+ mtq.MultiTermRewriteMethod = method;
try
{
multiSearcherDupls.Rewrite(mtq);
Assert.Fail("Should throw BooleanQuery.TooManyClauses");
}
- catch (BooleanQuery.TooManyClausesException e)
+ catch (BooleanQuery.TooManyClausesException /*e*/)
{
+ // LUCENENET: The assertion below fails when Dynamic PGO is
enabled (by default in .NET 8+) which can inline
+ // some methods at runtime. This causes the stack trace to be
different than expected. Rather than forcing
+ // NoInlining on the method, we will just remove the
assertion. It should only matter that the exception is
+ // thrown, not where it is thrown from.
+ // Original comment and assertion below:
+
// Maybe remove this assert in later versions, when internal
API changes:
- Assert.AreEqual("CheckMaxClauseCount", new StackTrace(e,
false).GetFrames()[0].GetMethod().Name); //, "Should throw
BooleanQuery.TooManyClauses with a stacktrace containing
checkMaxClauseCount()");
+ // Assert.AreEqual("CheckMaxClauseCount", new StackTrace(e,
false).GetFrames()[0].GetMethod()?.Name, "Should throw
BooleanQuery.TooManyClauses with a stacktrace containing
checkMaxClauseCount()");
Review Comment:
Alright, this case we can let pass.
I wish this had used the `StackTraceHelper` so it would be clear that this
is done in several places the same way (that is, declaring certain methods with
`NoInlining` to ensure the tests can function). This one was missed during the
sweep to apply `NoInlining` because `StackTraceHelper` was not used, which is
why it intermittently failed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
