-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Add BWC test to reveal #12895 #12912
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We couldthink of recreating older backwards indexes, too (at least for 9.x series).
|
||
// Gives you time to copy the index out!: (there is also | ||
// a test option to not remove temp dir...): | ||
Thread.sleep(100000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this must go away, please add nocommit
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought -Ptests.bwcdir=
made this sleep hack unnecessary? Seems like we shouldn't have any of these data generative tests using Thread.sleep
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for review @uschindler @benwtrent !
I closed this since I saw @mikemccand did similar things and his dictionary is much smaller than mine :)
this must go away, please add nocommit.
I'm very new to this part. I copied this pattern from another existing test . I can open another PR to clean this up if we are sure it makes no sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wwwwuuuuuuaaaaaaah. This must go away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I raised #12914
Oh thanks for working on this @gf2121 -- sorry that we both did it at the same time! |
... but at least it lead to discovering a horrifying Can we ban |
Solr used that, as solr is no longer part of our tree we could add sleep to fobiddenaps (maybe globally for both tests and main code). Maybe Lucene does not sleep in tests, so it could be an easy PR. Let's investigate. 🫣 |
Awesome, I opened #12946 for this. |
This patch get a 40000-terms subset (~200KB) of
wikibigall
terms dict, which can reproduce #12895. I have verified #12900 can fix the expection.