dsmiley commented on code in PR #4738:
URL: https://github.com/apache/solr/pull/4738#discussion_r3827008733
##########
dev-docs/admin-ui-tests.md:
##########
Review Comment:
Please remove the license header for dev-docs things. RAT will not complain
because no "release" contains it.
##########
solr/test-framework/src/java/org/apache/solr/embedded/JettyConfig.java:
##########
@@ -37,6 +37,9 @@ public class JettyConfig {
public final boolean enableV2;
public final boolean enableGracefulShutdown;
+ /** If true, serve the Admin UI static files and index.html like the
production web.xml does. */
+ public final boolean enableAdminUi;
+
private JettyConfig(
Review Comment:
This constructor has gotten out of control; only second to the worst
offender -- NodeConfig. It's only private so maybe we shouldn't care much but
maybe JettyConfig shouldn't be immutable, or should have a freeze().
##########
solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java:
##########
@@ -0,0 +1,534 @@
+/*
+ * 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.webapp;
+
+import com.carrotsearch.randomizedtesting.ThreadFilter;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.File;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.BooleanSupplier;
+import java.util.function.Function;
+import java.util.logging.Level;
+import org.apache.lucene.tests.util.QuickPatchThreadsFilter;
+import org.apache.lucene.util.SuppressForbidden;
+import org.apache.solr.SolrIgnoredThreadsFilter;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.EnvUtils;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.embedded.JettyConfig;
+import org.apache.solr.embedded.JettySolrRunner;
+import org.apache.solr.util.ExternalPaths;
+import org.eclipse.jetty.ee10.servlet.ServletHolder;
+import org.junit.AfterClass;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.rules.TestRule;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+import org.openqa.selenium.By;
+import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.OutputType;
+import org.openqa.selenium.StaleElementReferenceException;
+import org.openqa.selenium.TakesScreenshot;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebDriverException;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.chrome.ChromeDriver;
+import org.openqa.selenium.chrome.ChromeOptions;
+import org.openqa.selenium.logging.LogEntry;
+import org.openqa.selenium.logging.LogType;
+import org.openqa.selenium.logging.LoggingPreferences;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Base class for browser-based tests of the AngularJS Admin UI.
+ *
+ * <p>Starts a {@link SolrCloudTestCase} mini-cluster whose Jetty nodes also
serve the Admin UI
+ * static files (see {@code JettyConfig.Builder#enableAdminUi(boolean)}), then
drives the UI with a
+ * headless Chrome via Selenium WebDriver.
+ *
+ * <p>The tests require a locally installed Chrome/Chromium browser. Discovery
order: the {@code
+ * tests.ui.chrome.binary} system property, the {@code CHROME_BIN} environment
variable, then a list
+ * of well-known install locations. When no browser is found, all tests in the
class are skipped via
+ * {@link Assume}. The matching chromedriver is provisioned by Selenium
Manager, which may download
+ * it on first use (cached under {@code ~/.cache/selenium}); if that fails
(e.g. offline), tests are
+ * likewise skipped.
+ */
[email protected](bugUrl = "Admin UI browser tests drive plain http")
+@ThreadLeakFilters(
+ defaultFilters = true,
+ filters = {
+ SolrIgnoredThreadsFilter.class,
+ QuickPatchThreadsFilter.class,
+ AdminUiTestBase.WebDriverThreadsFilter.class
+ })
+@ThreadLeakLingering(linger = 5000)
+public abstract class AdminUiTestBase extends SolrCloudTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ protected static final Duration WAIT_TIMEOUT = Duration.ofSeconds(15);
+
+ protected static WebDriver driver;
+
+ /** Base url of the first node, e.g. {@code http://127.0.0.1:PORT/solr} */
+ protected static String baseUrl;
+
+ /**
+ * Optional security.json for the cluster. Subclasses assign this in their
{@code @BeforeClass}
+ * (which runs after this class's browser-starting one, but before the
cluster starts lazily on
+ * first use). Never assign it in a {@code static} block: test runners may
load all test classes
+ * up front, so static initializers of one class can run long before its
suite executes.
+ */
+ protected static String securityJson;
+
+ /**
+ * When true (set by {@code AdminUiStandaloneTestBase}), no cloud cluster is
started; the test
+ * class starts its own standalone {@link JettySolrRunner}(s), assigns
{@link #standaloneJetty}
+ * and {@link #baseUrl}, and stops them again.
+ */
+ protected static boolean standaloneMode = false;
+
+ /** The standalone node backing {@link #adminApi} when {@link
#standaloneMode} is set. */
+ protected static JettySolrRunner standaloneJetty;
+
+ /**
+ * Serves a minimal stand-in for the generated js-client bundle ({@code
libs/solr/index.js}),
+ * which only exists inside the built webapp, not in the source tree tests
serve from. The
+ * AngularJS {@code CollectionsV2} service fails to instantiate without the
{@code solrApi}
+ * global, taking the whole Collections screen down with it. Only the small
API surface the
+ * AngularJS UI actually uses is stubbed.
+ */
+ public static class StubJsClientServlet extends HttpServlet {
Review Comment:
This looks like a growing inadequacy, causing a roadblock the moment we use
the jsclient further in the admin UI. Surely we can serve the actual js-client
bundle.
##########
solr/webapp/build.gradle:
##########
@@ -43,6 +54,39 @@ dependencies {
if (gradle.ext.withUiModule) {
generatedUIBundle project(path: ":solr:ui", configuration:
"wasmJsUIBundle")
}
+
+ // Browser-based tests of the AngularJS Admin UI (see src/test)
+ testImplementation project(':solr:core')
+ testImplementation project(':solr:solrj')
+ testImplementation project(':solr:test-framework')
+ // puts the /sql handler on the in-JVM server classpath so the SQL screen
works
+ testRuntimeOnly project(':solr:modules:sql')
+ testImplementation libs.carrotsearch.randomizedtesting.runner
+ testImplementation libs.eclipse.jetty.ee10.servlet
+ testImplementation libs.jakarta.servlet.api
+ testImplementation libs.junit.junit
+ testImplementation(libs.selenium.chromedriver, {
+ // solr-core already provides the opentelemetry api/sdk that selenium's
optional
+ // tracing uses; the remaining excludes are compile-time-only annotation
jars
+ exclude group: 'io.opentelemetry'
+ exclude group: 'io.opentelemetry.instrumentation'
+ exclude group: 'io.opentelemetry.semconv'
+ exclude group: 'com.google.auto.service'
+ exclude group: 'org.jspecify'
Review Comment:
JSpecify is a great thing I think all Java projects should embrace. If you
remove this exclusion; what happens?
##########
solr/licenses/selenium-LICENSE-ASL.txt:
##########
Review Comment:
do we actually need licenses for dependencies not included in binary
releases?
##########
solr/test-framework/src/java/org/apache/solr/embedded/JettySolrRunner.java:
##########
Review Comment:
TBH I'd rather see a containerized Solr and we use that instead of extending
JettyConfig with this, including any maintenance burden of matching
web.xml/jetty configuration matters. But we don't *quite* have that.
##########
solr/webapp/src/test/org/apache/solr/webapp/AdminUiTestBase.java:
##########
@@ -0,0 +1,534 @@
+/*
+ * 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.webapp;
+
+import com.carrotsearch.randomizedtesting.ThreadFilter;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakFilters;
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakLingering;
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.File;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.time.Duration;
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.BooleanSupplier;
+import java.util.function.Function;
+import java.util.logging.Level;
+import org.apache.lucene.tests.util.QuickPatchThreadsFilter;
+import org.apache.lucene.util.SuppressForbidden;
+import org.apache.solr.SolrIgnoredThreadsFilter;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.EnvUtils;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.embedded.JettyConfig;
+import org.apache.solr.embedded.JettySolrRunner;
+import org.apache.solr.util.ExternalPaths;
+import org.eclipse.jetty.ee10.servlet.ServletHolder;
+import org.junit.AfterClass;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.rules.TestRule;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
+import org.openqa.selenium.By;
+import org.openqa.selenium.NoSuchElementException;
+import org.openqa.selenium.OutputType;
+import org.openqa.selenium.StaleElementReferenceException;
+import org.openqa.selenium.TakesScreenshot;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.WebDriverException;
+import org.openqa.selenium.WebElement;
+import org.openqa.selenium.chrome.ChromeDriver;
+import org.openqa.selenium.chrome.ChromeOptions;
+import org.openqa.selenium.logging.LogEntry;
+import org.openqa.selenium.logging.LogType;
+import org.openqa.selenium.logging.LoggingPreferences;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Base class for browser-based tests of the AngularJS Admin UI.
+ *
+ * <p>Starts a {@link SolrCloudTestCase} mini-cluster whose Jetty nodes also
serve the Admin UI
+ * static files (see {@code JettyConfig.Builder#enableAdminUi(boolean)}), then
drives the UI with a
+ * headless Chrome via Selenium WebDriver.
+ *
+ * <p>The tests require a locally installed Chrome/Chromium browser. Discovery
order: the {@code
+ * tests.ui.chrome.binary} system property, the {@code CHROME_BIN} environment
variable, then a list
+ * of well-known install locations. When no browser is found, all tests in the
class are skipped via
+ * {@link Assume}. The matching chromedriver is provisioned by Selenium
Manager, which may download
+ * it on first use (cached under {@code ~/.cache/selenium}); if that fails
(e.g. offline), tests are
+ * likewise skipped.
+ */
[email protected](bugUrl = "Admin UI browser tests drive plain http")
+@ThreadLeakFilters(
+ defaultFilters = true,
+ filters = {
+ SolrIgnoredThreadsFilter.class,
+ QuickPatchThreadsFilter.class,
+ AdminUiTestBase.WebDriverThreadsFilter.class
+ })
+@ThreadLeakLingering(linger = 5000)
+public abstract class AdminUiTestBase extends SolrCloudTestCase {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ protected static final Duration WAIT_TIMEOUT = Duration.ofSeconds(15);
+
+ protected static WebDriver driver;
+
+ /** Base url of the first node, e.g. {@code http://127.0.0.1:PORT/solr} */
+ protected static String baseUrl;
+
+ /**
+ * Optional security.json for the cluster. Subclasses assign this in their
{@code @BeforeClass}
+ * (which runs after this class's browser-starting one, but before the
cluster starts lazily on
+ * first use). Never assign it in a {@code static} block: test runners may
load all test classes
+ * up front, so static initializers of one class can run long before its
suite executes.
+ */
+ protected static String securityJson;
+
+ /**
+ * When true (set by {@code AdminUiStandaloneTestBase}), no cloud cluster is
started; the test
+ * class starts its own standalone {@link JettySolrRunner}(s), assigns
{@link #standaloneJetty}
+ * and {@link #baseUrl}, and stops them again.
+ */
+ protected static boolean standaloneMode = false;
+
+ /** The standalone node backing {@link #adminApi} when {@link
#standaloneMode} is set. */
+ protected static JettySolrRunner standaloneJetty;
+
+ /**
+ * Serves a minimal stand-in for the generated js-client bundle ({@code
libs/solr/index.js}),
+ * which only exists inside the built webapp, not in the source tree tests
serve from. The
+ * AngularJS {@code CollectionsV2} service fails to instantiate without the
{@code solrApi}
+ * global, taking the whole Collections screen down with it. Only the small
API surface the
+ * AngularJS UI actually uses is stubbed.
+ */
+ public static class StubJsClientServlet extends HttpServlet {
+ @Override
+ protected void doGet(HttpServletRequest req, HttpServletResponse resp)
throws IOException {
+ resp.setContentType("text/javascript");
+ resp.getOutputStream()
+ .write(
+ ("var solrApi = {\n"
Review Comment:
Lets use multiline strings; we're on Java 21!
--
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]
