mlbiscoc commented on PR #3330: URL: https://github.com/apache/solr/pull/3330#issuecomment-2835459509
> @mlbiscoc @psalagnac How about something like [this](https://github.com/kotman12/solr/commit/aaa7564cb049bcedf5472617aac1681c3fe31c2b) to test this? I don't _love_ it because it depends on an implementation detail of `CoreContainer::shutdown`, namely that the optimized election node deletion occurs _before_ the closing of caches. However, this fix addresses implementation details of that very method so I don't see how you can avoid relying on _some_ detail while also having a deterministic test. Ah this is pretty clever. Although I agree, I am not a big fan of this test being fixed in that closing of caches must happen after otherwise this whole test fails. Might confuse someone thinking something like this was intentional even though it is kind of a hack to test something not entirely relating to it. If we go with this approach, it should be commented in at the minimum this is the reason so if it ever fails due to the closing of caches moving, someone can at least understand why. I was also curious on looking into if Mockito.spy was possible and verify the calls for zkclient.delete() actually happening. Not sure how possible this is though with lots of these objects being nested deep from coreContainer and some being private/protected we would need to inject the spy one way or another. Maybe through java reflection? -- 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: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
