epugh commented on code in PR #5323:
URL: https://github.com/apache/solr/pull/5323#discussion_r4550250952
##########
dev-docs/writing-tests.adoc:
##########
@@ -0,0 +1,173 @@
+= Writing Tests for Solr
+:toc: left
+:toclevels: 2
+
+Which kind of test to write, and the lifecycle conventions they all share.
Building and running the suite is covered in xref:solr-source-code.adoc[Working
With Solr Source Code]; `./gradlew helpTests` covers running them.
+
+The rule: *write the cheapest kind of test that can actually catch the bug.*
+
+== Test types
+
+[cols="1,2,1,1", options="header"]
+|===
+| Base class | Covers | Cost | Status
+
+| `SolrTestCase`
Review Comment:
I have been begging for this for a long time.....
##########
solr/core/src/java/org/apache/solr/cli/SolrProcessManager.java:
##########
@@ -54,19 +54,37 @@ public class SolrProcessManager {
// Set this to true during testing to allow the SolrProcessManager to find
only mock Solr
// processes
public static boolean enableTestingMode = false;
+
+ /**
+ * System property ({@code solr.process.scan.marker}, or the {@code
SOLR_PROCESS_SCAN_MARKER}
Review Comment:
do you see this being useful for non CATS based testing too?
##########
dev-docs/writing-tests.adoc:
##########
@@ -0,0 +1,173 @@
+= Writing Tests for Solr
+:toc: left
+:toclevels: 2
+
+Which kind of test to write, and the lifecycle conventions they all share.
Building and running the suite is covered in xref:solr-source-code.adoc[Working
With Solr Source Code]; `./gradlew helpTests` covers running them.
+
+The rule: *write the cheapest kind of test that can actually catch the bug.*
+
+== Test types
+
+[cols="1,2,1,1", options="header"]
+|===
+| Base class | Covers | Cost | Status
+
+| `SolrTestCase`
+| Plain unit tests. Add `@ClassRule SolrJettyTestRule` (or
`EmbeddedSolrServerTestRule`) when you need a live node.
+| ms to seconds
+| Current. The default — extend this unless you need more.
+
+| `SolrCloudTestCase`
+| In-JVM `MiniSolrCloudCluster`: N Jetty nodes plus a `ZkTestServer`. Routing,
replication, recovery, Collections API, cluster state.
+| seconds to minutes
+| Current, but not a default.
+
+| `SolrShellTestCase`
+| CATS — Cross-platform Automated Testing System: `bin/solr` and
`bin\solr.cmd` as real subprocesses through the platform's own shell. Harness
in `solr/test-framework/src/java/org/apache/solr/shell/`, suites in
`solr/packaging/src/test/`.
+| seconds to minutes
+| Current. New CLI coverage goes here.
+
+| `AdminUiTestBase`
+| AngularJS Admin UI in headless Chrome via Selenium, against a 2-node cluster
whose Jetty nodes also serve the UI. In
`solr/webapp/src/test/org/apache/solr/webapp/`.
+| cluster + a browser
+| Current, opt-in with `-Ptests.selenium=true`. See
link:admin-ui-tests.md[admin-ui-tests.md].
+
+| A Testcontainers `@ClassRule`
+| An external service the test needs a real instance of — Tika Server, Kafka,
an S3 mock. Any base class; the container comes from a rule, not from
inheritance.
+| cluster + a container pull
+| Current. Skips itself when Docker is absent.
+
+| `SolrTestCaseJ4`
+| Adds the single-core `TestHarness` to `SolrTestCase`: `initCore`, `assertQ`,
`assertU`, SSL randomization. Directly extended by ~600 test classes and
transitively the base of ~1250 — four in five Solr tests, mostly via
`SolrCloudTestCase`.
+| as `SolrTestCase`
+| Deprecated. Not in new tests.
+
+| BATS, `solr/packaging/test/*.bats`
+| The original bash `bin/solr` tests. `./gradlew integrationTests`.
+| seconds to minutes, sequential
+| Deprecated in favour of CATS, which runs the same coverage on both shells.
Bash-only; never ran on Windows.
+|===
+
+`configureCluster(n)` starts `n` Jetty servers and an embedded ZooKeeper in
the test JVM before your first assertion, and every collection then goes
through the real Collections API with real leader election — which is why
`SolrCloudTestCase` is not the default, and why most cluster-test flakiness is
a missing `waitForState`.
+
+`SolrTestCaseJ4` is not annotated `@Deprecated` — too much extends it for that
to be useful — but it is on the way out. Keep editing existing J4 tests in
place; do not convert them as a side effect of an unrelated fix. New classes
extend `SolrTestCase` and build documents and params from plain SolrJ types.
+
+== Choosing a test type
+
+* Plain class, parser, config object, response writer, utility —
`SolrTestCase`, no server.
+* Needs a live Solr, one node — `SolrTestCase` + `SolrJettyTestRule`, or
`EmbeddedSolrServerTestRule` if the HTTP layer is not part of the test.
+* Only reproduces across nodes, or involves ZooKeeper, routing, replication,
recovery or the Collections API — `SolrCloudTestCase`, with the lowest node
count that still shows the bug.
+* In `bin/solr`, `bin/solr.cmd`, `solr.in.sh`/`solr.in.cmd`, CLI parsing,
output or exit codes — `SolrShellTestCase`. The only harness that catches a
Windows-only script bug.
+* Needs a real external service, not a mock — a Testcontainers `@ClassRule`;
see below.
+* In the Admin UI's JavaScript or templates — `AdminUiTestBase`. Test the APIs
it calls separately; keep the browser test to what the browser renders.
+* Tempted to add a `.bats` file — don't. Write it as a `SolrShellTestCase`.
Touch the `.bats` files only to keep them working until they are ported.
+
+== Randomized testing
+
+Every test that extends `SolrTestCase` runs under randomizedtesting's
`RandomizedRunner`, inherited from `LuceneTestCase`. One master seed per run
drives every `random()` call, so a failure is reproducible from the seed the
runner prints.
+
+The framework already varies Lucene codec and postings format, directory
implementation, merge policy, locale, time zone, and the order of test methods
— so no test may depend on method order, on the default locale, or on the
default time zone. SSL is randomized too, but only for `SolrTestCaseJ4`
subclasses, which is where `@RandomizeSSL` sits; opt out with `@SuppressSSL`.
+
+What to use:
+
+* `random()` — the per-test `Random`. Never `new Random()` or `Math.random()`:
an unseeded source makes the failure unreproducible.
+* `atLeast(n)` — `n * RANDOM_MULTIPLIER`, plus up to 50% random fudge. The
normal way to size a loop.
+* `rarely()` / `usually()` — take an unusual branch about 1% of the time (5%
in nightly runs).
+* `TestUtil.randomSimpleString(random())` and its neighbours for field values.
+
+`RANDOM_MULTIPLIER` is 2 in a nightly run and 1 otherwise, so `atLeast` and
`rarely` scale themselves; read `TEST_NIGHTLY` directly only to skip or resize
work those two cannot express.
+
+Never commit `@Seed`: it pins the master seed and throws away the coverage you
just added. `@Repeat` is for chasing a rare seed while you work, not for
keeping.
+
+Randomize when the input space is large and uniform, correctness does not
depend on the particular value — field values, document and shard counts, the
order of unrelated operations — and you can state the property without
reimplementing the logic: a round trip, an invariant, or a comparison against a
simpler reference or a control core.
+
+== @Rule and lifecycle
+
+`SolrTestCase` already installs, for every subclass:
`SystemPropertiesRestoreRule` at both class and method scope,
`LogLevelTestRule` (honours `@LogLevel`), `RevertDefaultThreadHandlerRule`,
`QueryLimitsTestInjectionRule`, an `ObjectReleaseTracker` check,
`@ThreadLeakFilters` with `SolrIgnoredThreadsFilter` +
`QuickPatchThreadsFilter`, `@ThreadLeakLingering(linger = 1000)`, and
`@SuppressSysoutChecks` (which is why Solr tests may log freely to the
console). Do not re-add any of it, and do not restore system properties by hand.
+
+Do:
+
+* Order rules with `RuleChain.outerRule(a).around(b)` in one field, as
`SolrTestCase` itself does. Separate `@Rule` fields are applied in an
undefined, reflection-dependent order unless you set `@Rule(order = n)` (JUnit
4.13+), which Solr does not use.
+* Use a `TestRule` rather than `@After` when cleanup must know whether the
test failed. Worked example: `SolrShellTestCase.shellSessionRule` snapshots
`SOLR_HOME` and the command transcript on a throwable, before the harness stops
the node. A rule also sees the `Description`, which is how
`@EnabledOnShell`/`@EnabledOnCli` skip individual CATS tests.
+* Prefer a `@ClassRule` `ExternalResource` over `@BeforeClass`/`@AfterClass`
pairs for anything that must be closed — that is what `SolrJettyTestRule` and
`EmbeddedSolrServerTestRule` are.
+* In `SolrCloudTestCase`, configure the cluster in `@BeforeClass`; the base
class's `@AfterClass` shuts it down. For per-test isolation use `@After
cluster.deleteAllCollections()` (which removes aliases too), not a cluster per
method.
+* Call `super.setUp()`/`super.tearDown()` if you override them, and use
`createTempDir()` for scratch space.
+* Mute expected ERRORs: `try (ErrorLogMuter errors =
ErrorLogMuter.substring("...")) { ... }`.
+
+Don't:
+
+* Don't set up in a `static` initializer — runners may load all test classes
long before a suite runs.
+* Don't assume `@AfterClass` sees a complete fixture: JUnit 4 runs it even
when `@BeforeClass` threw or raised an assumption failure.
+* Don't add a thread to `SolrIgnoredThreadsFilter` instead of fixing the leak;
anything listed there must be completely stateless. `@ThreadLeakLingering` is
resolved the same way, so a subclass declaration replaces the inherited 1000
rather than raising it — and is for a slow-but-correct shutdown only.
+* Don't redeclare `@ThreadLeakFilters` without repeating the inherited filters
— the annotation is not additive. `AdminUiTestBase` shows the correct form.
+* Don't leak tracked objects. `ObjectReleaseTracker` is asserted empty only
when the suite passed, so `ObjectTracker found N object(s) that were not
released` means your test left a `SolrCore`, `SolrIndexSearcher`, `Directory`
or similar open.
+
+Annotations:
+
+These are Lucene annotations wired up as randomizedtesting *test groups*,
disabled by default, so annotating a test with one stops it running in ordinary
builds and in PR CI:
+
+* `@Nightly` — correct, but too slow to run every time.
+* `@AwaitsFix(bugUrl = "...")` — known-broken, and the required bug URL is
what keeps it discoverable. Prefer it to `@Ignore`, which is plain JUnit and
invisible to group filters; use `@Ignore` only where nothing is broken, such as
an abstract base the runner would otherwise pick up.
+* `@LuceneTestCase.Monster(value)` — needs unusual disk or RAM.
+
+There is no `@Slow`, and `@BadApple` is gone: slow is `@Nightly`, broken is
`@AwaitsFix`. Read solr-source-code.adoc's guidance on frequently-failing tests
before disabling anything.
+
+== Containers for external services
Review Comment:
i am sure you are already planning this, but all of this test guidence could
be it's own PR (if you are breaking this big guy up!)....
##########
solr/packaging/src/test/org/apache/solr/shell/AdminConsoleUrlsTest.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import org.junit.Test;
+
+/**
+ * Port of {@code test_adminconsole_urls.bats}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on.
+ * Where it differs: each test starts its own node, as the original does,
because the last one needs
+ * a system property on the command line; the original's repeated {@code curl}
calls against one URL
+ * (one for the status code, one for the headers, one for the body) collapse
into a single {@code
+ * http()} request whose status, headers and body are all asserted; and every
{@code solr start} and
+ * every header assertion also asserts the status, so a 404 cannot pass as a
missing header.
+ */
+public class AdminConsoleUrlsTest extends SolrShellTestCase {
+
+ @Test
+ public void assertAbleToLaunchSolrAdminConsole() {
+ run("solr start");
+
+
http("/solr/").assertStatus(200).assertHeaderContains("text/html").assertContains("<html");
+
+ http("/solr/css/angular/chosen.css")
+ .assertStatus(200)
+ .assertHeaderContains("text/css")
+ .assertContains("{");
+ }
+
+ @Test
+ public void assertAdminConsoleStaticAssetsAreRevalidatedAndVersioned() {
+ run("solr start");
+
+ http("/solr/libs/solr/index.js")
+ .assertStatus(200)
+ .assertHeaderContains("Cache-Control: no-cache");
+
+
http("/solr/").assertStatus(200).assertNotMatches("(src|href)=\"[^\"]+\\.(js|css)\"");
+ }
+
+ @Test
+ public void assertCspHeaderContainsCustomConnectSrcUrls() {
+ // The original quotes the comma-separated value for the shell; unquoted
it is one token on bash
+ // and solr.cmd rejoins the pieces cmd splits on commas, so no quoting is
needed either way.
+ run(
+ "solr start -Dsolr.ui.headers.csp.connect-src.urls="
Review Comment:
would it make sense to relax wrapping so this was all one line? Just for
the copy/pasting debugging steps?
##########
solr/bin/solr.cmd:
##########
@@ -32,6 +32,15 @@ pushd %SOLR_TIP%
set SOLR_TIP=%CD%
popd
+REM Where the per-node files recording running Solr instances live. Same
variable and default as
Review Comment:
love all of this. Also the kind of finicky logic paths taht ai deals with
better than I ever could..
##########
solr/packaging/src/test/org/apache/solr/shell/AuthTest.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import org.apache.lucene.util.IOUtils;
+import org.apache.solr.shell.exec.Shell;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Port of {@code test_auth.bats}. There are further auth tests in {@link
SslTest}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on.
+ * Where it differs:
+ *
+ * <ul>
+ * <li>{@code curl} becomes {@link #http}, which now also carries
credentials, {@code curl -u
+ * user:password --basic} being half of what this suite does.
+ * <li>{@code solr auth enable} rewrites the include file, by default the
distribution's own and
+ * so shared with every concurrent class, so {@link #cleanSetup} points
{@code SOLR_INCLUDE}
+ * at a private copy — as {@code SslTest} does.
+ * <li>The original's {@code setup} runs {@code solr auth disable} before
every test, to undo what
+ * a previous test left in the include file and in ZooKeeper. Here the
include file is a
+ * pristine copy per test and {@code SOLR_HOME} — which holds the
embedded ZooKeeper's data —
+ * is wiped per test, so there is nothing to disable.
+ * <li>The two {@code auth enable} calls the original lets succeed-or-fail
silently are asserted
+ * to succeed.
+ * </ul>
+ */
+public class AuthTest extends SolrShellTestCase {
+
+ @Before
+ public void cleanSetup() throws IOException {
+ // BATS' common_clean_setup: every test here starts its own Solr, from a
clean SOLR_HOME. The
+ // wipe also discards the embedded ZooKeeper's data, and with it any
security.json.
+ IOUtils.rm(harnessEnv.solrHome());
+ Files.createDirectories(harnessEnv.logsDir());
+
+ // 'solr auth enable' appends to the include file, which by default
belongs to the distribution
+ // every test class shares, so each test gets a private copy in its own
work directory. The
+ // name has to keep its extension: bin\solr.cmd CALLs the include file.
+ String includeFile = cli.shell() == Shell.BASH ? "solr.in.sh" :
"solr.in.cmd";
+ env(
+ "SOLR_INCLUDE",
+
copy(harnessEnv.solrTip().resolve("bin").resolve(includeFile).toString(),
includeFile)
+ .toString());
+ }
+
+ @Test
+ public void authRejectsBlockUnknownOptionWithInvalidBoolean() {
+ runFail("solr auth enable --type basicAuth --credentials any:any
--block-unknown ture")
+ .assertContains("Argument [block-unknown] must be either true or
false, but was [ture]");
+ }
+
+ @Test
+ public void authRejectsUpdateIncludeFileOnlyOptionWithInvalidBoolean() {
+ runFail(
+ "solr auth enable --type basicAuth --credentials any:any
--update-include-file-only ture")
+ .assertContains(
+ "Argument [update-include-file-only] must be either true or false,
but was [ture]");
+ }
+
+ @Test
+ public void authEnableDisableLifecycle() {
+ run("solr start");
+ run("solr auth enable --type basicAuth --credentials name:password");
+ run("solr assert --started http://localhost:${SOLR_PORT} --timeout 5000");
+
+ http(
+
"/solr/admin/collections?action=CREATE&collection.configName=_default&name=test"
+ +
"&numShards=2&replicationFactor=1&router.name=compositeId&wt=json",
+ "name",
+ "password")
+ .assertContains("\"status\":0");
+
+ run("solr auth disable");
+ http("/solr/test/select?q=*:*").assertContains("\"numFound\":0");
+ run("solr stop --all");
Review Comment:
how does the interesection of `--all` impact the multi parapllelism? or is
that where the process marker stuff kicks in? I would be fine if in CATS you
have to name each port to avoid `--all` if we think that is at all brittle.
##########
solr/packaging/src/test/org/apache/solr/shell/AssertTest.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import org.junit.Test;
+
+/**
+ * Port of {@code test_assert.bats}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on. The
+ * original starts Solr inside each test rather than in {@code setup_file} —
one node user-managed,
+ * one in cloud mode — so each test here starts its own too; the harness stops
it when the test
+ * ends, which is the original's {@code teardown}.
+ */
+public class AssertTest extends SolrShellTestCase {
+
+ @Test
+ public void assertForNonCloudMode() {
+ run("solr assert --not-started http://localhost:${SOLR_PORT} --timeout
5000");
Review Comment:
explainer mode could fill in the `${SOLR_PORT} `;-)
##########
solr/packaging/src/test/org/apache/solr/shell/CompressionTest.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import java.util.Map;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Port of {@code test_compression.bats}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on.
+ * Where it differs:
+ *
+ * <ul>
+ * <li>{@code curl -D -} becomes {@link #http}, so the header dump the
original asserted on as
+ * part of the body is asserted on as the response headers instead.
+ * <li>{@code curl --compressed} becomes a request that asks for {@code
Accept-Encoding}; the
+ * harness decompresses the body the same way curl does.
+ * <li>The original's {@code solr start -e films} is deliberately not used:
on Windows {@code
+ * RunExampleTool#startSolr} drops every {@code SOLR_*} variable from
the spawned node's
+ * environment, so the node cannot be discovered or stopped and the
suite would be bash-only
+ * for a reason unrelated to what it tests (see {@code SnapshotsTest}).
These tests only need
+ * a collection with enough documents that a response exceeds Jetty's
2048-byte {@code
+ * minGzipSize}, so a plain start, {@code solr create} and {@code solr
post} of the example
+ * docs stand in.
+ * <li>The first two tests also assert the status, and the second and third
that the negotiated
+ * encoding really was gzip — the claim their names make.
+ * </ul>
+ */
+public class CompressionTest extends SolrShellTestCase {
+
+ private static final String COLLECTION = "compression_docs";
+ private static final String QUERY = "/solr/" + COLLECTION +
"/select?q=*:*&rows=100";
+
+ @BeforeClass
+ public static void startSolr() {
+ // BATS' setup_file, which started the films example purely to have
documents to query.
+ classCli.run("solr start");
+ classCli.run("solr create -c " + COLLECTION + " -d _default");
+ // Filtered to xml, json and csv so the Extract handler is not invoked. 45
documents, whose
+ // response is well over the 2048 bytes Jetty wants before it will
compress one at all.
+ classCli
+ .run(
+ "solr post --filetypes xml,json,csv --solr-url
http://localhost:${SOLR_PORT}";
Review Comment:
i wonder if multiline java would make this more readable, but then you
convert in backend to single line to execute?
##########
dev-docs/writing-tests.adoc:
##########
@@ -0,0 +1,173 @@
+= Writing Tests for Solr
+:toc: left
+:toclevels: 2
+
+Which kind of test to write, and the lifecycle conventions they all share.
Building and running the suite is covered in xref:solr-source-code.adoc[Working
With Solr Source Code]; `./gradlew helpTests` covers running them.
+
+The rule: *write the cheapest kind of test that can actually catch the bug.*
+
+== Test types
+
+[cols="1,2,1,1", options="header"]
+|===
+| Base class | Covers | Cost | Status
+
+| `SolrTestCase`
+| Plain unit tests. Add `@ClassRule SolrJettyTestRule` (or
`EmbeddedSolrServerTestRule`) when you need a live node.
+| ms to seconds
+| Current. The default — extend this unless you need more.
+
+| `SolrCloudTestCase`
+| In-JVM `MiniSolrCloudCluster`: N Jetty nodes plus a `ZkTestServer`. Routing,
replication, recovery, Collections API, cluster state.
+| seconds to minutes
+| Current, but not a default.
+
+| `SolrShellTestCase`
+| CATS — Cross-platform Automated Testing System: `bin/solr` and
`bin\solr.cmd` as real subprocesses through the platform's own shell. Harness
in `solr/test-framework/src/java/org/apache/solr/shell/`, suites in
`solr/packaging/src/test/`.
+| seconds to minutes
+| Current. New CLI coverage goes here.
+
+| `AdminUiTestBase`
+| AngularJS Admin UI in headless Chrome via Selenium, against a 2-node cluster
whose Jetty nodes also serve the UI. In
`solr/webapp/src/test/org/apache/solr/webapp/`.
+| cluster + a browser
+| Current, opt-in with `-Ptests.selenium=true`. See
link:admin-ui-tests.md[admin-ui-tests.md].
+
+| A Testcontainers `@ClassRule`
+| An external service the test needs a real instance of — Tika Server, Kafka,
an S3 mock. Any base class; the container comes from a rule, not from
inheritance.
+| cluster + a container pull
+| Current. Skips itself when Docker is absent.
+
+| `SolrTestCaseJ4`
+| Adds the single-core `TestHarness` to `SolrTestCase`: `initCore`, `assertQ`,
`assertU`, SSL randomization. Directly extended by ~600 test classes and
transitively the base of ~1250 — four in five Solr tests, mostly via
`SolrCloudTestCase`.
+| as `SolrTestCase`
+| Deprecated. Not in new tests.
+
+| BATS, `solr/packaging/test/*.bats`
+| The original bash `bin/solr` tests. `./gradlew integrationTests`.
+| seconds to minutes, sequential
+| Deprecated in favour of CATS, which runs the same coverage on both shells.
Bash-only; never ran on Windows.
+|===
+
+`configureCluster(n)` starts `n` Jetty servers and an embedded ZooKeeper in
the test JVM before your first assertion, and every collection then goes
through the real Collections API with real leader election — which is why
`SolrCloudTestCase` is not the default, and why most cluster-test flakiness is
a missing `waitForState`.
+
+`SolrTestCaseJ4` is not annotated `@Deprecated` — too much extends it for that
to be useful — but it is on the way out. Keep editing existing J4 tests in
place; do not convert them as a side effect of an unrelated fix. New classes
extend `SolrTestCase` and build documents and params from plain SolrJ types.
+
+== Choosing a test type
+
+* Plain class, parser, config object, response writer, utility —
`SolrTestCase`, no server.
+* Needs a live Solr, one node — `SolrTestCase` + `SolrJettyTestRule`, or
`EmbeddedSolrServerTestRule` if the HTTP layer is not part of the test.
+* Only reproduces across nodes, or involves ZooKeeper, routing, replication,
recovery or the Collections API — `SolrCloudTestCase`, with the lowest node
count that still shows the bug.
+* In `bin/solr`, `bin/solr.cmd`, `solr.in.sh`/`solr.in.cmd`, CLI parsing,
output or exit codes — `SolrShellTestCase`. The only harness that catches a
Windows-only script bug.
+* Needs a real external service, not a mock — a Testcontainers `@ClassRule`;
see below.
+* In the Admin UI's JavaScript or templates — `AdminUiTestBase`. Test the APIs
it calls separately; keep the browser test to what the browser renders.
+* Tempted to add a `.bats` file — don't. Write it as a `SolrShellTestCase`.
Touch the `.bats` files only to keep them working until they are ported.
+
+== Randomized testing
+
+Every test that extends `SolrTestCase` runs under randomizedtesting's
`RandomizedRunner`, inherited from `LuceneTestCase`. One master seed per run
drives every `random()` call, so a failure is reproducible from the seed the
runner prints.
+
+The framework already varies Lucene codec and postings format, directory
implementation, merge policy, locale, time zone, and the order of test methods
— so no test may depend on method order, on the default locale, or on the
default time zone. SSL is randomized too, but only for `SolrTestCaseJ4`
subclasses, which is where `@RandomizeSSL` sits; opt out with `@SuppressSSL`.
+
+What to use:
+
+* `random()` — the per-test `Random`. Never `new Random()` or `Math.random()`:
an unseeded source makes the failure unreproducible.
+* `atLeast(n)` — `n * RANDOM_MULTIPLIER`, plus up to 50% random fudge. The
normal way to size a loop.
+* `rarely()` / `usually()` — take an unusual branch about 1% of the time (5%
in nightly runs).
+* `TestUtil.randomSimpleString(random())` and its neighbours for field values.
+
+`RANDOM_MULTIPLIER` is 2 in a nightly run and 1 otherwise, so `atLeast` and
`rarely` scale themselves; read `TEST_NIGHTLY` directly only to skip or resize
work those two cannot express.
+
+Never commit `@Seed`: it pins the master seed and throws away the coverage you
just added. `@Repeat` is for chasing a rare seed while you work, not for
keeping.
+
+Randomize when the input space is large and uniform, correctness does not
depend on the particular value — field values, document and shard counts, the
order of unrelated operations — and you can state the property without
reimplementing the logic: a round trip, an invariant, or a comparison against a
simpler reference or a control core.
+
+== @Rule and lifecycle
+
+`SolrTestCase` already installs, for every subclass:
`SystemPropertiesRestoreRule` at both class and method scope,
`LogLevelTestRule` (honours `@LogLevel`), `RevertDefaultThreadHandlerRule`,
`QueryLimitsTestInjectionRule`, an `ObjectReleaseTracker` check,
`@ThreadLeakFilters` with `SolrIgnoredThreadsFilter` +
`QuickPatchThreadsFilter`, `@ThreadLeakLingering(linger = 1000)`, and
`@SuppressSysoutChecks` (which is why Solr tests may log freely to the
console). Do not re-add any of it, and do not restore system properties by hand.
+
+Do:
+
+* Order rules with `RuleChain.outerRule(a).around(b)` in one field, as
`SolrTestCase` itself does. Separate `@Rule` fields are applied in an
undefined, reflection-dependent order unless you set `@Rule(order = n)` (JUnit
4.13+), which Solr does not use.
+* Use a `TestRule` rather than `@After` when cleanup must know whether the
test failed. Worked example: `SolrShellTestCase.shellSessionRule` snapshots
`SOLR_HOME` and the command transcript on a throwable, before the harness stops
the node. A rule also sees the `Description`, which is how
`@EnabledOnShell`/`@EnabledOnCli` skip individual CATS tests.
+* Prefer a `@ClassRule` `ExternalResource` over `@BeforeClass`/`@AfterClass`
pairs for anything that must be closed — that is what `SolrJettyTestRule` and
`EmbeddedSolrServerTestRule` are.
+* In `SolrCloudTestCase`, configure the cluster in `@BeforeClass`; the base
class's `@AfterClass` shuts it down. For per-test isolation use `@After
cluster.deleteAllCollections()` (which removes aliases too), not a cluster per
method.
+* Call `super.setUp()`/`super.tearDown()` if you override them, and use
`createTempDir()` for scratch space.
+* Mute expected ERRORs: `try (ErrorLogMuter errors =
ErrorLogMuter.substring("...")) { ... }`.
+
+Don't:
+
+* Don't set up in a `static` initializer — runners may load all test classes
long before a suite runs.
+* Don't assume `@AfterClass` sees a complete fixture: JUnit 4 runs it even
when `@BeforeClass` threw or raised an assumption failure.
+* Don't add a thread to `SolrIgnoredThreadsFilter` instead of fixing the leak;
anything listed there must be completely stateless. `@ThreadLeakLingering` is
resolved the same way, so a subclass declaration replaces the inherited 1000
rather than raising it — and is for a slow-but-correct shutdown only.
+* Don't redeclare `@ThreadLeakFilters` without repeating the inherited filters
— the annotation is not additive. `AdminUiTestBase` shows the correct form.
+* Don't leak tracked objects. `ObjectReleaseTracker` is asserted empty only
when the suite passed, so `ObjectTracker found N object(s) that were not
released` means your test left a `SolrCore`, `SolrIndexSearcher`, `Directory`
or similar open.
+
+Annotations:
+
+These are Lucene annotations wired up as randomizedtesting *test groups*,
disabled by default, so annotating a test with one stops it running in ordinary
builds and in PR CI:
+
+* `@Nightly` — correct, but too slow to run every time.
+* `@AwaitsFix(bugUrl = "...")` — known-broken, and the required bug URL is
what keeps it discoverable. Prefer it to `@Ignore`, which is plain JUnit and
invisible to group filters; use `@Ignore` only where nothing is broken, such as
an abstract base the runner would otherwise pick up.
+* `@LuceneTestCase.Monster(value)` — needs unusual disk or RAM.
+
+There is no `@Slow`, and `@BadApple` is gone: slow is `@Nightly`, broken is
`@AwaitsFix`. Read solr-source-code.adoc's guidance on frequently-failing tests
before disabling anything.
+
+== Containers for external services
+
+When a test needs a real instance of something Solr talks to, start it with
Testcontainers from a JUnit rule rather than by hand. Three examples in tree,
all an `ExternalResource` declared as `@ClassRule`: `TikaServerContainerRule`
(extraction), `KafkaContainerRule` (cross-dc-manager), `S3MockContainerRule`
(s3-repository). `TikaServerExtractionBackendTest` is the one to copy.
+
+* `@ClassRule`, not `@Rule` — a container pull and start is far too expensive
per method.
+* `Assume.assumeTrue(.., DockerClientFactory.instance().isDockerAvailable())`
in `before()`, so the test skips where Docker is absent instead of failing.
+* `.waitingFor(Wait.forListeningPort())`, so there is no polling loop and no
sleep.
+* `getMappedPort(n)` for the address. Never publish a fixed host port: it
collides with whatever else the machine is running, and with another test class
in a parallel build.
+
+== Waiting, not sleeping
+
+A fixed `Thread.sleep` is paid on every run and still flakes under load,
because the thing you are waiting for is slower on a busy machine, not faster.
Wait for the condition instead:
+
+* Best: have the other side signal. Complete a `CompletableFuture` when it
reaches the point you care about and `get(timeout)` it from the test thread.
+* If you must poll, use the wait your harness already has — `TimeOut#waitFor`
(solr-core), `cluster.waitForState(..)` in `SolrCloudTestCase`,
`awaitOrFail(..)` in `SolrShellTestCase` — rather than a hand-rolled loop.
+* Derive a timeout from whatever it bounds, and name it: `ABORT_DEADLINE_MS =
SlowServlet.DELAY_MS / 2`, not a bare `2, SECONDS`.
+
+A sleep you cannot replace is worth a comment saying what it is waiting for,
because it usually marks a missing signal in the code under test rather than in
the test.
+
+== Worked examples
+
+`SolrTestCase`::
+`solr/core/src/test/org/apache/solr/core/backup/BackupIdTest.java` — 49 lines,
no server, no rules.
++
+`solr/core/src/test/org/apache/solr/handler/admin/api/NodeThreadsAPITest.java`
— 54 lines; the live-node shape without J4: `@ClassRule SolrJettyTestRule` +
`startSolr(createTempDir())`, driven through the generated SolrJ client.
+
+`SolrCloudTestCase`::
+`solr/core/src/test/org/apache/solr/HelloWorldSolrCloudTestCase.java` —
written as a teaching template, instructions in its javadoc.
++
+`solr/core/src/test/org/apache/solr/cloud/CollectionStateZnodeTest.java` — 74
lines; the real minimal shape, with `@After deleteAllCollections()` and both
`waitForState` forms.
+
+`SolrShellTestCase`::
+`solr/packaging/src/test/org/apache/solr/shell/ZkTest.java` — canonical BATS
port: `classCli.run(...)` for a class-lifetime node, fluent
`run(...).assertContains(...)`, `${SOLR_PORT}` expansion, portable stand-ins
for `touch` and `curl`.
++
+`solr/packaging/src/test/org/apache/solr/shell/PostTest.java` — same, plus
`assertContainsForCli(commonsCliText, picocliText)` for wording that differs
between the two CLI implementations.
+
+`AdminUiTestBase`::
+`solr/webapp/src/test/org/apache/solr/webapp/AdminUiDashboardTest.java` — 61
lines, one screen; the version and JVM values are compared against live
`/admin/info/system` JSON rather than against constants.
Review Comment:
do we want the number of lines? i guess they will be wrong soon, but maybe
that doesn't matter?
##########
solr/packaging/src/test-files/org/apache/solr/shell/website/index.html:
##########
Review Comment:
??? not sure I get why this file is hear, but maybe later on?
##########
solr/packaging/src/test/org/apache/solr/shell/AssertTest.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import org.junit.Test;
+
+/**
+ * Port of {@code test_assert.bats}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on. The
+ * original starts Solr inside each test rather than in {@code setup_file} —
one node user-managed,
+ * one in cloud mode — so each test here starts its own too; the harness stops
it when the test
+ * ends, which is the original's {@code teardown}.
+ */
+public class AssertTest extends SolrShellTestCase {
+
+ @Test
+ public void assertForNonCloudMode() {
+ run("solr assert --not-started http://localhost:${SOLR_PORT} --timeout
5000");
+ run("solr start --user-managed");
+ run("solr assert --started http://localhost:${SOLR_PORT} --timeout 5000");
+
+ run("solr assert --not-cloud http://localhost:${SOLR_PORT}/solr";)
+ .assertContains("needn't include Solr's context-root")
+ .assertNotContains("ERROR");
+
+ runFail("solr assert --cloud http://localhost:${SOLR_PORT}";)
+ .assertContains("ERROR: Solr is not running in cloud mode");
+
+ runFail("solr assert --cloud http://localhost:${SOLR_PORT} --exitcode");
+ }
+
+ @Test
+ public void assertForCloudMode() {
+ run("solr start");
+ run("solr assert --started http://localhost:${SOLR_PORT} --timeout 5000");
+
+ // Strengthened: the original only refuted "ERROR", which an empty output
satisfies without
Review Comment:
i suspect you will remove these extra "migration" comments before landing it
;-). I wish "`///`" meant that it wouldn't show up in the merged code!
##########
solr/packaging/src/test/org/apache/solr/shell/CompressionTest.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import java.util.Map;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Port of {@code test_compression.bats}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on.
+ * Where it differs:
+ *
+ * <ul>
+ * <li>{@code curl -D -} becomes {@link #http}, so the header dump the
original asserted on as
+ * part of the body is asserted on as the response headers instead.
+ * <li>{@code curl --compressed} becomes a request that asks for {@code
Accept-Encoding}; the
+ * harness decompresses the body the same way curl does.
+ * <li>The original's {@code solr start -e films} is deliberately not used:
on Windows {@code
+ * RunExampleTool#startSolr} drops every {@code SOLR_*} variable from
the spawned node's
Review Comment:
does this indicate that RunExampleTool needs some improvements based on what
we've learned in this?
##########
solr/packaging/src/test/org/apache/solr/shell/AdminConsoleUrlsTest.java:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import org.junit.Test;
+
+/**
+ * Port of {@code test_adminconsole_urls.bats}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on.
+ * Where it differs: each test starts its own node, as the original does,
because the last one needs
+ * a system property on the command line; the original's repeated {@code curl}
calls against one URL
+ * (one for the status code, one for the headers, one for the body) collapse
into a single {@code
+ * http()} request whose status, headers and body are all asserted; and every
{@code solr start} and
+ * every header assertion also asserts the status, so a 404 cannot pass as a
missing header.
+ */
+public class AdminConsoleUrlsTest extends SolrShellTestCase {
+
+ @Test
+ public void assertAbleToLaunchSolrAdminConsole() {
+ run("solr start");
+
+
http("/solr/").assertStatus(200).assertHeaderContains("text/html").assertContains("<html");
Review Comment:
one thing I liekd about bats was I could literally copy the lines and run
them against my own `bin/solr start` with a change or two for the variable
names to test thigns....
Could you run a CATS test with some sort of `--explainer-mode` where it
owuld output the equivalent curl command for the `http`? I often find
debugging needing to runt he steps myself... I suppose this COULD be run by
intellji debugger... But it would be nice to get the curl command quivalent
of the `http`'s....?
##########
solr/packaging/src/test/org/apache/solr/shell/CompressionTest.java:
##########
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import java.util.Map;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Port of {@code test_compression.bats}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on.
+ * Where it differs:
+ *
+ * <ul>
+ * <li>{@code curl -D -} becomes {@link #http}, so the header dump the
original asserted on as
+ * part of the body is asserted on as the response headers instead.
+ * <li>{@code curl --compressed} becomes a request that asks for {@code
Accept-Encoding}; the
+ * harness decompresses the body the same way curl does.
+ * <li>The original's {@code solr start -e films} is deliberately not used:
on Windows {@code
+ * RunExampleTool#startSolr} drops every {@code SOLR_*} variable from
the spawned node's
+ * environment, so the node cannot be discovered or stopped and the
suite would be bash-only
+ * for a reason unrelated to what it tests (see {@code SnapshotsTest}).
These tests only need
+ * a collection with enough documents that a response exceeds Jetty's
2048-byte {@code
+ * minGzipSize}, so a plain start, {@code solr create} and {@code solr
post} of the example
+ * docs stand in.
+ * <li>The first two tests also assert the status, and the second and third
that the negotiated
+ * encoding really was gzip — the claim their names make.
+ * </ul>
+ */
+public class CompressionTest extends SolrShellTestCase {
+
+ private static final String COLLECTION = "compression_docs";
Review Comment:
for these tests, i don't know that we need constants... They are
private... And they just add a smidge more to everything in terms of
thinking..... Line 55 would be easier to read if it was just `solr create -c
compression_docs -d _default` for example....
##########
solr/packaging/src/test/org/apache/solr/shell/AuthTest.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.shell;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import org.apache.lucene.util.IOUtils;
+import org.apache.solr.shell.exec.Shell;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Port of {@code test_auth.bats}. There are further auth tests in {@link
SslTest}.
+ *
+ * <p>Kept close to the original, including the order of the tests and the
text each asserts on.
+ * Where it differs:
+ *
+ * <ul>
+ * <li>{@code curl} becomes {@link #http}, which now also carries
credentials, {@code curl -u
+ * user:password --basic} being half of what this suite does.
+ * <li>{@code solr auth enable} rewrites the include file, by default the
distribution's own and
+ * so shared with every concurrent class, so {@link #cleanSetup} points
{@code SOLR_INCLUDE}
+ * at a private copy — as {@code SslTest} does.
+ * <li>The original's {@code setup} runs {@code solr auth disable} before
every test, to undo what
+ * a previous test left in the include file and in ZooKeeper. Here the
include file is a
+ * pristine copy per test and {@code SOLR_HOME} — which holds the
embedded ZooKeeper's data —
+ * is wiped per test, so there is nothing to disable.
+ * <li>The two {@code auth enable} calls the original lets succeed-or-fail
silently are asserted
+ * to succeed.
+ * </ul>
+ */
+public class AuthTest extends SolrShellTestCase {
+
+ @Before
+ public void cleanSetup() throws IOException {
+ // BATS' common_clean_setup: every test here starts its own Solr, from a
clean SOLR_HOME. The
+ // wipe also discards the embedded ZooKeeper's data, and with it any
security.json.
+ IOUtils.rm(harnessEnv.solrHome());
+ Files.createDirectories(harnessEnv.logsDir());
+
+ // 'solr auth enable' appends to the include file, which by default
belongs to the distribution
+ // every test class shares, so each test gets a private copy in its own
work directory. The
+ // name has to keep its extension: bin\solr.cmd CALLs the include file.
+ String includeFile = cli.shell() == Shell.BASH ? "solr.in.sh" :
"solr.in.cmd";
Review Comment:
should this type of logic just be part of the CATS framework? we know
SOLR_INCLUDE comes in two flavours.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
