Progress on SOLR-16573 EmbeddedSolrServerTestRule

[David Smiley <ds...@apache.org>]

Happy New Year everyone.

Joshua and I have some good progress on a new EmbeddedSolrServerTestRule
https://issues.apache.org/jira/browse/SOLR-16573 + PR (read the JIRA first,
always)
Keep in mind this is one incremental step, and I expect it will be
evolutionary (The API will evolve as we address more tests for more
scenarios).

Here's an example from RootFieldTest, which subclasses
EmbeddedSolrServerTestBase:

@BeforeClass
public static void beforeTest() throws Exception {
  useRootSchema = random().nextBoolean();
  // schema15.xml declares _root_ field, while schema-rest.xml does not.
  String schema = useRootSchema ? "schema15.xml" : "schema-rest.xml";
  SolrTestCaseJ4.newRandomConfig();
  solrClientTestRule
      .build()
      .setSolrHome(Path.of(SolrTestCaseJ4.TEST_HOME()))
      .useTempDataDir()
      .setSchemaFile(schema)
      .init();
}


The "set" needs to become "with" to match the builder style.
The ".build()" method instead of calling a constructor of an inner Builder
is debatable; I like this FWIW.  The constructor of this thing does
nothing; it's initialized later by the test using it as seen above.  The
rule is declared like:

@ClassRule
public static EmbeddedSolrServerTestRule solrClientTestRule = new
EmbeddedSolrServerTestRule();

I had initially wanted to initialize this thing at the spot it's declared
instead of having an instance that isn't yet initialized.  But it's common
for the construction to specify a bunch of things that might in turn access
the randomized context (e.g. LuceneTestCase.createTempDir() will,
surprisingly).  But for a statically declared field, the randomized context
doesn't exist yet in the lifecycle of a test.  So we use the builder
pattern post-construction for the initialization.

After Joshua converts some more tests, the PR will be ready.
Maybe EmbeddedSolrServerTestBase should be removed here as well, as it'll
do nothing other than declare this rule and a delegating method to get the
client.

After this issue, we should do an equivalent implementation for HTTP /
Jetty.

~ David Smiley
Apache Lucene/Solr Search Developer
http://www.linkedin.com/in/davidwsmiley

Re: Progress on SOLR-16573 EmbeddedSolrServerTestRule

[Jason Gerlowski <ge...@gmail.com>]

Hi David,

Thanks for bringing the PR up for attention here.  Had a few thoughts on
the setter and build() naming, as you called out, and commented on those in
the PR.  But otherwise, it looks very nice so far!

Best,

Jason

On Tue, Jan 3, 2023 at 1:06 AM David Smiley <ds...@apache.org> wrote:

> Happy New Year everyone.
>
> Joshua and I have some good progress on a new EmbeddedSolrServerTestRule
> https://issues.apache.org/jira/browse/SOLR-16573 + PR (read the JIRA
> first,
> always)
> Keep in mind this is one incremental step, and I expect it will be
> evolutionary (The API will evolve as we address more tests for more
> scenarios).
>
> Here's an example from RootFieldTest, which subclasses
> EmbeddedSolrServerTestBase:
>
> @BeforeClass
> public static void beforeTest() throws Exception {
>   useRootSchema = random().nextBoolean();
>   // schema15.xml declares _root_ field, while schema-rest.xml does not.
>   String schema = useRootSchema ? "schema15.xml" : "schema-rest.xml";
>   SolrTestCaseJ4.newRandomConfig();
>   solrClientTestRule
>       .build()
>       .setSolrHome(Path.of(SolrTestCaseJ4.TEST_HOME()))
>       .useTempDataDir()
>       .setSchemaFile(schema)
>       .init();
> }
>
>
> The "set" needs to become "with" to match the builder style.
> The ".build()" method instead of calling a constructor of an inner Builder
> is debatable; I like this FWIW.  The constructor of this thing does
> nothing; it's initialized later by the test using it as seen above.  The
> rule is declared like:
>
> @ClassRule
> public static EmbeddedSolrServerTestRule solrClientTestRule = new
> EmbeddedSolrServerTestRule();
>
> I had initially wanted to initialize this thing at the spot it's declared
> instead of having an instance that isn't yet initialized.  But it's common
> for the construction to specify a bunch of things that might in turn access
> the randomized context (e.g. LuceneTestCase.createTempDir() will,
> surprisingly).  But for a statically declared field, the randomized context
> doesn't exist yet in the lifecycle of a test.  So we use the builder
> pattern post-construction for the initialization.
>
> After Joshua converts some more tests, the PR will be ready.
> Maybe EmbeddedSolrServerTestBase should be removed here as well, as it'll
> do nothing other than declare this rule and a delegating method to get the
> client.
>
> After this issue, we should do an equivalent implementation for HTTP /
> Jetty.
>
> ~ David Smiley
> Apache Lucene/Solr Search Developer
> http://www.linkedin.com/in/davidwsmiley
>