Tests: TestLongNormValueSource, AssertingScorable - #14448
Conversation
AssortingScorable: don't wrap needlessly
github-project-automation
Bot
moved this to Open
in OpenSearch Lucene & Core Performance Tracking
|
|
||
| public static Scorable wrap(Scorable in) { | ||
| if (in instanceof AssertingScorable) { | ||
| if (in instanceof WrappedScorer || in instanceof AssertingScorable) { |
There was a problem hiding this comment.
I don't understand this part of the change, this method expects the returned Scorable to have assertions enabled, but it may not be the case if this is an instance of WrappedScorer?
There was a problem hiding this comment.
The comment below explains why a WrappedScorer is used sometimes. It doesn't make sense to have a WrappedScorer around a WrappedScorer (right?).
I encountered this issue while running the other test in a debugger. So much wrapping!
There was a problem hiding this comment.
Thanks for explaining, there can be much wrapping indeed. I had missed that AssertingScorable would sometimes do this, I opened a PR that hopefully cleans this up a bit. #14452
|
|
||
| public static Scorable wrap(Scorable in) { | ||
| if (in instanceof AssertingScorable) { | ||
| if (in instanceof WrappedScorer || in instanceof AssertingScorable) { |
There was a problem hiding this comment.
Thanks for explaining, there can be much wrapping indeed. I had missed that AssertingScorable would sometimes do this, I opened a PR that hopefully cleans this up a bit. #14452
github-project-automation
Bot
moved this from Open
to Merged
in OpenSearch Lucene & Core Performance Tracking
TestLongNormValueSource: improve AssortingScorable: don't wrap needlessly
2 participants
TestLongNormValueSource: test specifics
AssertingScorable: don't wrap needlessly