SOLR-13592: Introduce EmbeddedSolrTestBase for better integration tests

[thomaswoeckinger]

To allow better integration of EmbeddedSolrServer, hidden implementation should be moved out of SolrJettyTestBase

[thomaswoeckinger]

@gerlowskija plz review

[thomaswoeckinger]

@gerlowskija Any progress on this PR?

[thomaswoeckinger]

@gerlowskija Anything new?

[thomaswoeckinger]

Need any support, or are you just busy at the moment?

[thomaswoeckinger]

Still nothing?

[thomaswoeckinger]

@gerlowskija Are you still working on this?

[thomaswoeckinger]

Rebased again, @gerlowskija it is two weeks ago since last notice, maybe someone else could takeover the review if jason is to busy?

[thomaswoeckinger]

@gerlowskija i saw you are working on other issues too, so i hope you have time to get this PR done, if not so maybe you can hand over to another reviewer.

[gerlowskija]

My apologies Thomas, my gmail filters weren't letting through github notifications and I didn't see this until you pinged me in JIRA.

This all looks good to me, except for the whitespace formatting, which we've talked about on other PRs. Github makes it easy to hide introduction/removal of trailing whitespace, but other changes are harder to hide and make the diff noisier to read and harder to review with confidence: adding "final" modifiers to existing variables, breaking up long method signatures onto multiple lines, etc. Would you mind scrubbing some of those changes from this PR?

In the meantime, I'm going to start "beasting" (run {{ant test-help}} for info) this PR to make sure that none of the affected tests have any flaky failures. I'll comment again in a day or two with some results. (If you want to help out, you can also run some beast-runs if your machine allows it.)

Thanks for your continued patience.

[thomaswoeckinger]

My apologies Thomas, my gmail filters weren't letting through github notifications and I didn't see this until you pinged me in JIRA.

:-), no problem, i am still interested to get this in!
Do you know if there are any plans on the master branch migrating to junit 5, this would allow to write tests using default interfaces, so it would be very easy to let them run under different scenarios like embedded, jetty, cloud and so on.

This all looks good to me, except for the whitespace formatting, which we've talked about on other PRs. Github makes it easy to hide introduction/removal of trailing whitespace, but other changes are harder to hide and make the diff noisier to read and harder to review with confidence: adding "final" modifiers to existing variables, breaking up long method signatures onto multiple lines, etc. Would you mind scrubbing some of those changes from this PR?

Yes we talked about, my point: why using common format rules if even no one is using them.
But anyway, i can take a look at the method signatures, if you like, but i think it is not that difficult to review at the current state.

In the meantime, I'm going to start "beasting" (run {{ant test-help}} for info) this PR to make sure that none of the affected tests have any flaky failures. I'll comment again in a day or two with some results. (If you want to help out, you can also run some beast-runs if your machine allows it.)

Thanks for your continued patience.

i will have a look at it, the default test are all running on my site, i will comment on this PR when i have results.

And you are very welcome with your comments and suggestions!

[thomaswoeckinger]

Did not had time to test yet, i am at vacation for the next to week, than i can continue to work on it.

[gerlowskija]

I haven't heard anyone bring up JUnit 5, no. We'll have to do it eventually, but it could potentially be a massive change for Solr, so my guess is that no one will push for it until they absolutely have to.

[gerlowskija]

Is there any difference between LargeVolumeEmbeddedTest, LargeVolumeJettyTest, and LargeVolumeBinaryJettyTest? It looks like all three are exactly the same with this PR. They look suspiciously similar before this PR too, so I think the problem pre-exists this change. But it's still very odd. Just wondering if you dug into that at all as you were working?

[thomaswoeckinger]

Is there any difference between LargeVolumeEmbeddedTest, LargeVolumeJettyTest, and LargeVolumeBinaryJettyTest? It looks like all three are exactly the same with this PR. They look suspiciously similar before this PR too, so I think the problem pre-exists this change. But it's still very odd. Just wondering if you dug into that at all as you were working?

They are using old init from my point of view, all other Jetty tests are using initCore().

[thomaswoeckinger]

Any plans on merging this back to 8.x?

[thomaswoeckinger]

I will close the coresponding issue now.

[dsmiley]

Note that the javadocs on org.apache.solr.SolrJettyTestBase#createNewSolrClient is obsolete as it still references using an "embedded implementation" which is no longer true.

[thomaswoeckinger]

Note that the javadocs on org.apache.solr.SolrJettyTestBase#createNewSolrClient is obsolete as it still references using an "embedded implementation" which is no longer true.

I will remove the reference in the followup PR #665 .