You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "vogievetsky (via GitHub)" <gi...@apache.org> on 2026/10/09 22:09:39 UTC
[PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
vogievetsky opened a new pull request, #20544:
URL: https://github.com/apache/druid/pull/20544
## Note from the human
So the e2e tests in the web console have not been touched in a long time. On a whim I asked Claude "If I was added e2e tests today would they be written differently"... and boy did it have a lot to say.
While Claude was clauding I described what I was doing to @gianm and he said: "It would be cool if web console e2e tests used the embedded tests framework" and the rest is history...
## Description
The web console's end-to-end tests ran in CI by building a full Druid distribution, starting it with `script/druid`, running the Playwright specs against it and stopping it. That was slow, the specs shared one long-lived cluster (so leftovers from one test could break another), and the console flows lived apart from the Druid features they exercise.
This PR runs the specs on the embedded clusters that the rest of Druid's tests use (`embedded-tests`), modernizes the specs themselves, and adds console coverage for features that the embedded tests already cover through the API. Each console test lives next to the embedded tests of its feature (for example `s3/S3WebConsoleTest` next to `S3StorageTest`), so the "load data from S3" flow is tested through the console too.
#### Running the console e2e specs on embedded clusters
- `WebConsoleTestBase` (in `embedded-tests`, package `org.apache.druid.testing.embedded.console`) starts an embedded cluster with a Router, which serves the console from the `web-console` jar, and runs a Playwright spec of `web-console/e2e-tests` on it with `npx playwright test <spec>`. A subclass adds the resources the spec needs (S3 or Kafka containers, extensions, properties) in `addResources` / `configureCluster` and passes their settings to the spec as environment variables. The JUnit failure message carries Playwright's output; each spec writes its screenshots and traces to its own `web-console/test-results/<spec>/`.
- The tests are tagged `web-console`: excluded from the default surefire run and run by failsafe with the new `web-console-tests` profile:
```bash
./mvnw -pl web-console install -DskipTests # with the console built, for the Router to serve it
./mvnw -pl embedded-tests verify -Pweb-console-tests [-Dit.test=BatchIndexingWebConsoleTest]
```
- `-Dweb.console.keepAlive=true` keeps a test's cluster up and prints the command to run its spec against it (with `--ui` / `--debug` as you like). `-Dweb.console.port=18081` runs the specs against the dev server (`npm start`) instead of the bundled console.
- Every test class gets a fresh cluster, so the specs no longer clean up after themselves there (`DRUID_E2E_TEST_CLUSTER_IS_DISPOSABLE`).
- `WebConsoleSpecsTest` (a plain unit test) fails if a spec of `web-console/e2e-tests` isn't run by any `*WebConsoleTest`, or if one runs a spec that doesn't exist.
- The specs still run against a quickstart cluster with `npm run test-e2e` (`script/druid` stays for that); the ones that need a resource (S3, Kafka, extra extensions, a lookup table) skip themselves there.
#### CI
`.github/scripts/web-checks.sh` installs `web-console` (with the console built) and then runs `./mvnw verify -pl 'embedded-tests,!web-console' -am -Pweb-console-tests -DskipUTs`, which builds only the modules `embedded-tests` needs: no distribution, no extensions download, no separate JVMs to start. `worker.yml` uploads `embedded-tests/target/failsafe-reports/` (the Druid and Playwright logs) and `web-console/test-results/` instead of the distribution's logs, and the existing JUnit report step now shows each console test.
#### Modernized e2e specs
The specs were written in a Puppeteer style; they now use Playwright as intended:
- The `@playwright/test` runner (`playwright.config.ts`) instead of Jest: fixtures instead of per-spec browser setup, a `globalSetup` that waits for the cluster, `expect.poll` / `toPass` instead of a home-made retry, screenshots, traces and an `error-context.md` on failure.
- Locators (`getByRole`, form fields found by their label) instead of XPath and element handles.
- The cluster state (tasks finishing, segments loading, supervisor and Dart query states) is checked through SQL on the system tables rather than by reading view columns by position; views are read by their column headers.
- Test data is set up through the HTTP API rather than by shelling out to `post-index-task`.
- Page objects are plain functions and types (no `Object.assign` data classes), with `loadData(page, config)` for the data loader.
- `E2E_SHOW_STEPS=true` saves a screenshot of each step a test takes to `e2e-tests/steps/<spec>-NNN.png`, a quick way to see what a test does.
- `web-console/e2e-tests/README.md` describes the tests, how to run them both ways, and how to write one.
Running them on embedded clusters also surfaced three latent races in the specs that the quickstart's timing had hidden (reading a view's table before it loaded, cancelling a query before it was sent, reading the Query view's previous result), now fixed.
#### New console coverage
| Spec | Run by | What it covers |
|---|---|---|
| `s3-ingestion.spec.ts` | `s3.S3WebConsoleTest` | Data loader from S3 (an S3 container that is also deep storage) |
| `input-formats.spec.ts` | `indexer.InputFormatsWebConsoleTest` | Data loader with CSV, TSV, Parquet, ORC and Avro OCF files: the format it picks and the columns it parses |
| `kafka-ingestion.spec.ts` | `indexing.KafkaWebConsoleTest` | A Kafka supervisor through the data loader (a Kafka container), then suspend, resume and terminate from the Supervisors view |
| `sql-ingestion.spec.ts` | `msq.SqlIngestionWebConsoleTest` | The SQL data loader, and `REPLACE` from external data and from a datasource in the Query view |
| `datasource-actions.spec.ts` | `indexing.DatasourceActionsWebConsoleTest` | Mark a datasource's segments unused and used again, then delete them with a kill task |
| `retention-rules.spec.ts` | `server.RetentionRulesWebConsoleTest` | Editing retention rules (`loadForever`, then `dropForever`) |
| `jdbc-lookup.spec.ts` | `lookup.JdbcLookupWebConsoleTest` | Initializing lookups and adding a JDBC lookup (on a table in the Derby metadata store), then querying it |
| `dart.spec.ts` | `msq.DartWebConsoleTest` | Running a query with the Dart engine, the current Dart queries panel and its details, cancelling a running Dart query |
The existing specs (`tutorial-batch`, `reindexing`, `auto-compaction`, `multi-stage-query`, `cancel-query`) run from `indexing.BatchIndexingWebConsoleTest`, `compact.AutoCompactionWebConsoleTest`, `msq.MultiStageQueryWebConsoleTest` and `query.SqlQueryCancelWebConsoleTest`.
#### Web console fixes found on the way
- Table filters in the URL (`#datasources/datasource=...`) couldn't hold values with `#` or `?` (which end the hash route) or `|` (which separates values), so going from a datasource with one of those in its name to its tasks or segments filtered on the wrong thing. `#`, `?`, `&`, `/` and `%` are now encoded, and `\|` / `\\` escape `|` / `\` in a value.
- `Api.encodePath` left `^` and `|` as is, which Jetty rejects (400 Illegal Path Character), so every action on a datasource with one of those in its name (mark unused, kill, retention rules, compaction config) failed.
#### Release note
Fixed the web console's handling of datasource names containing `#`, `?`, `|`, `^` or `\`: links between views now filter on the right datasource, and actions such as marking segments unused, killing data, and editing retention rules or compaction configs no longer fail for such names.
<hr>
##### Key changed/added classes in this PR
* `WebConsoleTestBase`, `WebConsoleSpecsTest` and the `*WebConsoleTest` classes (`embedded-tests`)
* `embedded-tests/pom.xml` (`web-console-tests` profile), `.github/scripts/web-checks.sh`, `.github/workflows/worker.yml`
* `web-console/e2e-tests/**`, `web-console/playwright.config.ts`
* `TableFilter`, `TableFilters`, `Api.encodePath` (web console)
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added documentation for new or modified features or behaviors (`web-console/e2e-tests/README.md`).
- [x] a release note entry in the PR description.
- [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths (`table-filter.spec.ts`, `table-filters.spec.ts`, `hash-routing.spec.ts`, `api.spec.ts`).
- [x] added integration tests.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "vogievetsky (via GitHub)" <gi...@apache.org>.
vogievetsky commented on code in PR #20544:
URL: https://github.com/apache/druid/pull/20544#discussion_r4297117268
##########
web-console/src/utils/table-filters/table-filter.ts:
##########
@@ -64,14 +96,18 @@ export class TableFilter {
return new TableFilter(filter.id, mode, value);
}
+ /**
+ * @param value the values to filter on, or a needle (as typed in a filter input) of values separated by `|` (see
+ * `splitNeedle`). To filter on one literal value, that may contain a `|`, pass it as `[value]`.
+ */
constructor(key: string, mode: FilterMode, value: string | string[]) {
this.key = key;
this.mode = mode;
- this.values = typeof value === 'string' ? value.split('|') : value;
+ this.values = typeof value === 'string' ? TableFilter.splitNeedle(value) : value;
}
public get value(): string {
- return this.values.join('|');
+ return TableFilter.joinNeedle(this.values);
Review Comment:
Good catch, fixed in e7bc667479: the segment timeline now gets the filter's raw value (`values[0]`), only when there's exactly one.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #20544:
URL: https://github.com/apache/druid/pull/20544#discussion_r4325136212
##########
web-console/e2e-tests/README.md:
##########
@@ -0,0 +1,192 @@
+<!--
+ ~ 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.
+ -->
+
+# End-to-end tests
+
+These tests drive the web console in a real (headless) Chromium against a real Druid cluster. They catch what the unit
+tests can't: the console and Druid's APIs disagreeing, a flow across several views breaking, or a request that only
+fails against a live cluster.
+
+They use the Playwright test runner (`@playwright/test`, configured in `../playwright.config.ts`). The Chromium it
+drives is downloaded by the `playwright-chromium` package's install script, so keep the two packages on the same
+version. They run in CI on embedded clusters (see below), as part of `.github/scripts/web-checks.sh`.
+
+## Running them
+
+The tests need a Druid cluster with its Router on :8888 (every request goes through the console's service), running
+on the same machine (its tasks read the `examples/quickstart/tutorial` data files of this checkout):
+
+```bash
+script/druid build # once, builds a distribution with the extensions the tests need
+script/druid start
+npm run test-e2e # all the tests, one at a time (they share the cluster)
+script/druid stop
+```
+
+Before the tests start, a global setup (`util/global-setup.ts`) waits for the console's SQL endpoint to answer.
+
+- **One file**: `npm run test-e2e -- cancel-query` (any `playwright test` arguments go after `--`)
+- **See the browser**: `npm run test-e2e -- --headed`, step through a test with `--debug`, or pick and watch tests in
+ the UI mode with `--ui`
+- **Test a dev server instead of the bundled console**: `DRUID_E2E_TEST_UNIFIED_CONSOLE_PORT=18081 npm run test-e2e`.
+ Without it the tests run against the console that the router on :8888 serves, which was bundled when Druid was
+ built, not your working tree.
+- **On failure** a test leaves a screenshot, a trace (`npx playwright show-trace <path>`: every action with the DOM,
+ console and network at that point) and an `error-context.md` (the page as an accessibility tree) in `test-results/`
+ (git ignored). Every HTTP response with a status of 400 or more is also logged with its body
+ (`util/fixtures.ts`).
+- **See the steps a test takes**: `E2E_SHOW_STEPS=true npm run test-e2e`. Each test saves a screenshot at each of
+ its steps (each step of the data loader, each view it reads, a query's results, a dialog it fills in, and the
+ page as it ends) to `steps/<spec>-001.png`, `-002.png`, ... (git ignored, replaced by the next run of the spec).
+ The quickest way to see what a test does, especially one someone else wrote. It works on the embedded clusters
+ too: `E2E_SHOW_STEPS=true mvn -pl embedded-tests verify -Pweb-console-tests ...`.
+
+### On an embedded cluster (as in CI)
+
+Each spec is run by a `*WebConsoleTest` class in `embedded-tests`, in the package of the functionality it covers. The
+class starts an embedded cluster (in one JVM, with an in-memory metadata store, its Router on :8888) with the resources
+the spec needs, and runs the spec on it with `npx playwright test <spec>`:
+
+| Spec | Run by (in `org.apache.druid.testing.embedded`) |
+|-----------------------------------------------|-------------------------------------------------|
+| `tutorial-batch.spec.ts`, `reindexing.spec.ts` | `indexing.BatchIndexingWebConsoleTest` |
+| `auto-compaction.spec.ts` | `compact.AutoCompactionWebConsoleTest` |
+| `multi-stage-query.spec.ts` | `msq.MultiStageQueryWebConsoleTest` |
+| `cancel-query.spec.ts` | `query.SqlQueryCancelWebConsoleTest` |
+| `s3-ingestion.spec.ts` | `s3.S3WebConsoleTest` (with an S3 container) |
+| `input-formats.spec.ts` | `indexer.InputFormatsWebConsoleTest` |
+| `kafka-ingestion.spec.ts` | `indexing.KafkaWebConsoleTest` (with Kafka) |
+| `sql-ingestion.spec.ts` | `msq.SqlIngestionWebConsoleTest` |
+| `datasource-actions.spec.ts` | `indexing.DatasourceActionsWebConsoleTest` |
+| `retention-rules.spec.ts` | `server.RetentionRulesWebConsoleTest` |
+| `jdbc-lookup.spec.ts` | `lookup.JdbcLookupWebConsoleTest` |
+| `dart.spec.ts` | `msq.DartWebConsoleTest` |
+| `input-formats.spec.ts` | Loads the same 10 rows (over 2 days) of Wikipedia edits from local CSV, TSV, Parquet, ORC and Avro OCF files through the data loader, one test per format. Checks that the **Parse data** step picks the right input format by itself and parses the columns, and that the datasource ends up as 2 segments with 10 rows. Skipped without `DRUID_E2E_TEST_DATA_DIR` (the data files of embedded-tests): it runs on an embedded cluster with the Avro, Parquet and ORC extensions, from `InputFormatsWebConsoleTest`. |
Review Comment:
[P3] Keep the embedded-test table columns consistent
**Finding:** The table beginning at line 66 declares its second column as `Run by`, but lines 80-86 append long `What it does` descriptions as additional rows without a new header. These rows render in the wrong column and duplicate several spec entries, making the embedded-test mapping misleading.
**Suggestion:** Move the descriptions under a separate `Spec | What it does` table or remove the duplicate block so the embedded-cluster table contains only runner mappings.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #20544:
URL: https://github.com/apache/druid/pull/20544#discussion_r4235102493
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/indexing/KafkaWebConsoleTest.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.druid.testing.embedded.indexing;
+
+import org.apache.druid.indexing.kafka.simulate.KafkaResource;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+import org.apache.druid.testing.embedded.console.WebConsoleTestBase;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.zip.GZIPInputStream;
+
+/**
+ * Sets up a Kafka supervisor through the data loader of the web console, then suspends, resumes and terminates it
+ * from the Supervisors view.
+ */
+public class KafkaWebConsoleTest extends WebConsoleTestBase
+{
+ private static final String TOPIC = "wikipedia";
+
+ private final KafkaResource kafka = new KafkaResource();
+
+ @Override
+ protected void addResources(EmbeddedDruidCluster cluster)
+ {
+ cluster.addResource(kafka);
+ }
+
+ /**
+ * Publishes the edits of the tutorial file to the topic, one per message, in one partition (so in the order of the
+ * file), as in the Kafka tutorial.
+ */
+ @BeforeAll
+ public void publishData() throws Exception
+ {
+ final File dataFile = new File(webConsoleDir(), "../examples/quickstart/tutorial/wikiticker-2015-09-12-sampled.json.gz");
+ final List<byte[]> records = new ArrayList<>();
+ try (BufferedReader reader = new BufferedReader(
+ new InputStreamReader(new GZIPInputStream(new FileInputStream(dataFile)), StandardCharsets.UTF_8)
Review Comment:
## CodeQL / Potential input resource leak
This FileInputStream is not always closed on method exit.
[Show more details](https://github.com/apache/druid/security/code-scanning/12073)
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm merged PR #20544:
URL: https://github.com/apache/druid/pull/20544
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #20544:
URL: https://github.com/apache/druid/pull/20544#discussion_r4290000873
##########
web-console/src/utils/table-filters/table-filter.ts:
##########
@@ -64,14 +96,18 @@ export class TableFilter {
return new TableFilter(filter.id, mode, value);
}
+ /**
+ * @param value the values to filter on, or a needle (as typed in a filter input) of values separated by `|` (see
+ * `splitNeedle`). To filter on one literal value, that may contain a `|`, pass it as `[value]`.
+ */
constructor(key: string, mode: FilterMode, value: string | string[]) {
this.key = key;
this.mode = mode;
- this.values = typeof value === 'string' ? value.split('|') : value;
+ this.values = typeof value === 'string' ? TableFilter.splitNeedle(value) : value;
}
public get value(): string {
- return this.values.join('|');
+ return TableFilter.joinNeedle(this.values);
Review Comment:
[P2] Keep literal datasource names raw for timeline consumers
**Finding:** The new getter serializes literal values for the table-filter grammar (`a|b` becomes `a\|b`). However, the existing Datasources and Segments views still pass `filter.value` directly to SegmentTimeline as a datasource name. With the new literal representation, a datasource containing `|` or `\` is sent to the timeline with its escape characters, so the SQL/API timeline query cannot find the real datasource.
**Suggestion:** Use the raw single value (`filter.values[0]`) at those timeline call sites, or expose separate raw-value and serialized-needle accessors so consumers do not treat the escaped grammar value as a datasource identifier.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "vogievetsky (via GitHub)" <gi...@apache.org>.
vogievetsky commented on code in PR #20544:
URL: https://github.com/apache/druid/pull/20544#discussion_r4408809580
##########
web-console/e2e-tests/README.md:
##########
@@ -0,0 +1,192 @@
+<!--
+ ~ 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.
+ -->
+
+# End-to-end tests
+
+These tests drive the web console in a real (headless) Chromium against a real Druid cluster. They catch what the unit
+tests can't: the console and Druid's APIs disagreeing, a flow across several views breaking, or a request that only
+fails against a live cluster.
+
+They use the Playwright test runner (`@playwright/test`, configured in `../playwright.config.ts`). The Chromium it
+drives is downloaded by the `playwright-chromium` package's install script, so keep the two packages on the same
+version. They run in CI on embedded clusters (see below), as part of `.github/scripts/web-checks.sh`.
+
+## Running them
+
+The tests need a Druid cluster with its Router on :8888 (every request goes through the console's service), running
+on the same machine (its tasks read the `examples/quickstart/tutorial` data files of this checkout):
+
+```bash
+script/druid build # once, builds a distribution with the extensions the tests need
+script/druid start
+npm run test-e2e # all the tests, one at a time (they share the cluster)
+script/druid stop
+```
+
+Before the tests start, a global setup (`util/global-setup.ts`) waits for the console's SQL endpoint to answer.
+
+- **One file**: `npm run test-e2e -- cancel-query` (any `playwright test` arguments go after `--`)
+- **See the browser**: `npm run test-e2e -- --headed`, step through a test with `--debug`, or pick and watch tests in
+ the UI mode with `--ui`
+- **Test a dev server instead of the bundled console**: `DRUID_E2E_TEST_UNIFIED_CONSOLE_PORT=18081 npm run test-e2e`.
+ Without it the tests run against the console that the router on :8888 serves, which was bundled when Druid was
+ built, not your working tree.
+- **On failure** a test leaves a screenshot, a trace (`npx playwright show-trace <path>`: every action with the DOM,
+ console and network at that point) and an `error-context.md` (the page as an accessibility tree) in `test-results/`
+ (git ignored). Every HTTP response with a status of 400 or more is also logged with its body
+ (`util/fixtures.ts`).
+- **See the steps a test takes**: `E2E_SHOW_STEPS=true npm run test-e2e`. Each test saves a screenshot at each of
+ its steps (each step of the data loader, each view it reads, a query's results, a dialog it fills in, and the
+ page as it ends) to `steps/<spec>-001.png`, `-002.png`, ... (git ignored, replaced by the next run of the spec).
+ The quickest way to see what a test does, especially one someone else wrote. It works on the embedded clusters
+ too: `E2E_SHOW_STEPS=true mvn -pl embedded-tests verify -Pweb-console-tests ...`.
+
+### On an embedded cluster (as in CI)
+
+Each spec is run by a `*WebConsoleTest` class in `embedded-tests`, in the package of the functionality it covers. The
+class starts an embedded cluster (in one JVM, with an in-memory metadata store, its Router on :8888) with the resources
+the spec needs, and runs the spec on it with `npx playwright test <spec>`:
+
+| Spec | Run by (in `org.apache.druid.testing.embedded`) |
+|-----------------------------------------------|-------------------------------------------------|
+| `tutorial-batch.spec.ts`, `reindexing.spec.ts` | `indexing.BatchIndexingWebConsoleTest` |
+| `auto-compaction.spec.ts` | `compact.AutoCompactionWebConsoleTest` |
+| `multi-stage-query.spec.ts` | `msq.MultiStageQueryWebConsoleTest` |
+| `cancel-query.spec.ts` | `query.SqlQueryCancelWebConsoleTest` |
+| `s3-ingestion.spec.ts` | `s3.S3WebConsoleTest` (with an S3 container) |
+| `input-formats.spec.ts` | `indexer.InputFormatsWebConsoleTest` |
+| `kafka-ingestion.spec.ts` | `indexing.KafkaWebConsoleTest` (with Kafka) |
+| `sql-ingestion.spec.ts` | `msq.SqlIngestionWebConsoleTest` |
+| `datasource-actions.spec.ts` | `indexing.DatasourceActionsWebConsoleTest` |
+| `retention-rules.spec.ts` | `server.RetentionRulesWebConsoleTest` |
+| `jdbc-lookup.spec.ts` | `lookup.JdbcLookupWebConsoleTest` |
+| `dart.spec.ts` | `msq.DartWebConsoleTest` |
+| `input-formats.spec.ts` | Loads the same 10 rows (over 2 days) of Wikipedia edits from local CSV, TSV, Parquet, ORC and Avro OCF files through the data loader, one test per format. Checks that the **Parse data** step picks the right input format by itself and parses the columns, and that the datasource ends up as 2 segments with 10 rows. Skipped without `DRUID_E2E_TEST_DATA_DIR` (the data files of embedded-tests): it runs on an embedded cluster with the Avro, Parquet and ORC extensions, from `InputFormatsWebConsoleTest`. |
Review Comment:
Right, those rows had landed in the wrong table; fixed in 2b78e93aba: they're now at the end of the table of what each test does, and the embedded-cluster table only maps specs to the classes that run them.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on code in PR #20544:
URL: https://github.com/apache/druid/pull/20544#discussion_r4237633384
##########
web-console/e2e-tests/component/workbench/overview.ts:
##########
@@ -16,43 +16,181 @@
* limitations under the License.
*/
-import type * as playwright from 'playwright-chromium';
+import type { Locator, Page, Request } from '@playwright/test';
-import { clickButton, setQueryInput } from '../../util/playwright';
+import { expect } from '../../util/fixtures';
+import { clickButton, clickMenuItem, openView, setQueryInput } from '../../util/playwright';
+import { showStep } from '../../util/steps';
import { extractTable } from '../../util/table';
/**
- * Represents the workbench tab.
+ * How to run a query.
+ */
+export interface QueryOptions {
+ /** The engine to pick in the view's engine menu, by its name there (like "SQL (Dart)"), rather than "Auto" */
+ readonly engine?: string;
+}
+
+/**
+ * What the view sent to run a query.
+ */
+export interface SubmittedQuery {
+ /** The engine of the query's context, which the view sets for the engine picked (like "msq-dart") */
+ readonly engine?: string;
+ /** The ID that the view gives the query (SQL queries, natively or with Dart), to cancel it */
+ readonly sqlQueryId?: string;
+}
+
+/**
+ * Represents the Query view (the workbench).
*/
export class WorkbenchOverview {
- private readonly page: playwright.Page;
- private readonly baseUrl: string;
+ private readonly page: Page;
+
+ /** What the view sent to run the last query run (or started) */
+ lastSubmittedQuery: SubmittedQuery | undefined;
- constructor(page: playwright.Page, unifiedConsoleUrl: string) {
+ constructor(page: Page) {
this.page = page;
- this.baseUrl = unifiedConsoleUrl + '#workbench';
}
- async runQuery(query: string): Promise<string[][]> {
- await this.page.goto(this.baseUrl);
- await this.page.reload({ waitUntil: 'networkidle' });
+ /**
+ * Runs a query and returns its results (as text, one array of cells per row). Throws if the query fails.
+ */
+ async runQuery(query: string, options: QueryOptions = {}): Promise<string[][]> {
+ const results = this.page.locator('.result-table-pane');
+ await this.run(query, options, results);
+ await showStep(this.page, 'Query view: the query results');
+ return await extractTable(results);
+ }
+
+ /**
+ * Runs an ingestion query (INSERT or REPLACE, run as an MSQ task) and returns what the view says when it is done
+ * (like "39,244 rows inserted into ..."). Throws if the query fails.
+ */
+ async runIngestQuery(query: string, options: QueryOptions = {}): Promise<string> {
+ const ingestSuccess = this.page.locator('.ingest-success-pane');
+ await this.run(query, options, ingestSuccess);
+ await showStep(this.page, 'Query view: the data ingested');
+ return await ingestSuccess.innerText();
+ }
+
+ /**
+ * Starts a query, without waiting for it to finish, and returns what the view sent to run it.
+ */
+ async startQuery(query: string, options: QueryOptions = {}): Promise<SubmittedQuery> {
+ await this.submit(query, options);
+ await showStep(this.page, 'Query view: the query running');
+ return this.lastSubmittedQuery!;
+ }
+
+ private async run(query: string, options: QueryOptions, done: Locator): Promise<void> {
+ await this.submit(query, options);
+
+ const error = this.page.locator('.execution-error-pane');
+ await done.or(error).waitFor({ timeout: QUERY_TIMEOUT });
Review Comment:
[P1] Wait for the current ingestion result
**Finding:** The workbench keeps the previous successful execution visible while a new query loads, so `done.or(error).waitFor()` can resolve on the old `.ingest-success-pane`. The second `runIngestQuery` in `sql-ingestion.spec.ts` can therefore return the first query's text before the new task finishes.
**Suggestion:** Wait for the current execution to complete or verify a query-specific state transition before reading the result pane.
##########
web-console/e2e-tests/component/load-data/sql-data-loader.ts:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.
+ */
+
+import type { Page } from '@playwright/test';
+
+import { clickButton, getLabeledInput, openView, setLabeledInput } from '../../util/playwright';
+import { showStep } from '../../util/steps';
+import { waitForTaskSlots } from '../workbench/overview';
+
+import type { DataConnector } from './data-connector';
+import { connectorCardTitle, fillConnector } from './data-connector';
+
+/**
+ * What to set at each step of the SQL data loader ("Load data" > "Batch - SQL"), which ingests with an MSQ task.
+ */
+export interface SqlDataLoaderConfig {
+ readonly connector: DataConnector;
+ /** Checks the input format that the data loader picked and the columns it parsed */
+ readonly validateParseData?: (parsed: { inputFormat: string; columns: string[] }) => void;
+ /** The new datasource to create */
+ readonly datasourceName: string;
+}
+
+/**
+ * Goes through each step of the SQL data loader, starts the ingestion and waits for it to finish. Throws if it fails.
+ */
+export async function loadDataWithSql(page: Page, config: SqlDataLoaderConfig): Promise<void> {
+ await openView(page, 'sql-data-loader');
+
+ // Select input type
+ await page
+ .locator('.input-source-step .bp6-card')
+ .filter({ has: page.locator('p', { hasText: connectorCardTitle(config.connector) }) })
+ .click();
+ await fillConnector(page, config.connector);
+ await showStep(page, `SQL data loader: ${connectorCardTitle(config.connector)}`);
+ await clickButton(page, 'Connect data');
+
+ // Parse
+ const parseDataTable = page.locator('.input-format-step .parse-data-table');
+ await parseDataTable.locator('.column-name').first().waitFor();
+ if (config.validateParseData) {
+ config.validateParseData({
+ inputFormat: await getLabeledInput(page, 'Input format'),
+ columns: await parseDataTable.locator('.column-name').allTextContents(),
+ });
+ }
+ await showStep(page, 'SQL data loader: Parse');
+ await clickButton(page.locator('.input-format-step .prev-next-bar'), 'Next');
+
+ // Schema, with the destination (a new datasource)
+ const schemaStep = page.locator('.schema-step');
+ await schemaStep.locator('.destination-button').click();
+ const destinationDialog = page.locator('.destination-dialog');
+ await clickButton(destinationDialog, 'New table');
+ await setLabeledInput(destinationDialog, 'New table name', config.datasourceName);
+ await showStep(page, 'SQL data loader: Destination');
+ await clickButton(destinationDialog, 'Save');
+ await destinationDialog.waitFor({ state: 'detached' });
+ await showStep(page, 'SQL data loader: Schema');
+ await clickButton(schemaStep.locator('.prev-next-bar'), 'Start loading data');
Review Comment:
[P1] Use the SQL loader's rendered submit label
**Finding:** The schema step renders the exact button name `Start ingesting data`, but this helper still searches for `Start loading data`. Because `clickButton` uses an exact accessible-name match, `sql-ingestion.spec.ts` times out before starting ingestion, so `SqlIngestionWebConsoleTest` cannot pass at this head.
**Suggestion:** Update the helper to the rendered label, or revert the temporary UI rename before merging.
##########
web-console/src/utils/table-filters/table-filter.ts:
##########
@@ -64,14 +96,18 @@ export class TableFilter {
return new TableFilter(filter.id, mode, value);
}
+ /**
+ * @param value the values to filter on, or a needle (as typed in a filter input) of values separated by `|` (see
+ * `splitNeedle`). To filter on one literal value, that may contain a `|`, pass it as `[value]`.
+ */
constructor(key: string, mode: FilterMode, value: string | string[]) {
this.key = key;
this.mode = mode;
- this.values = typeof value === 'string' ? value.split('|') : value;
+ this.values = typeof value === 'string' ? TableFilter.splitNeedle(value) : value;
Review Comment:
[P2] Preserve literal pipes in datasource filters
**Finding:** String values are now parsed with `splitNeedle`, where an unescaped `|` means OR. Existing datasource links in `segments-view.tsx` and `datasources-view.tsx` still pass datasource names as strings, so a valid datasource named `a|b` becomes two filter values instead of one literal datasource.
**Suggestion:** Use the constructor's literal-value form for those datasource links and add a pipe-containing datasource regression case.
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/console/WebConsoleTestBase.java:
##########
@@ -0,0 +1,231 @@
+/*
+ * 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.druid.testing.embedded.console;
+
+import org.apache.druid.guice.SleepModule;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.query.aggregation.datasketches.hll.HllSketchModule;
+import org.apache.druid.query.aggregation.datasketches.quantiles.DoublesSketchModule;
+import org.apache.druid.query.aggregation.datasketches.theta.SketchModule;
+import org.apache.druid.testing.embedded.EmbeddedBroker;
+import org.apache.druid.testing.embedded.EmbeddedCoordinator;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+import org.apache.druid.testing.embedded.EmbeddedHistorical;
+import org.apache.druid.testing.embedded.EmbeddedIndexer;
+import org.apache.druid.testing.embedded.EmbeddedOverlord;
+import org.apache.druid.testing.embedded.EmbeddedRouter;
+import org.apache.druid.testing.embedded.junit5.EmbeddedClusterTestBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Tag;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Base class for the web console end-to-end tests: a cluster with a Router (which serves the web console), on which
+ * the tests run the Playwright specs of {@code web-console/e2e-tests}.
+ * <p>
+ * Steps to write a test:
+ * <ul>
+ * <li>Write the Playwright spec in {@code web-console/e2e-tests}.</li>
+ * <li>Write a {@code *WebConsoleTest} class that extends this class, in the package of the functionality the spec
+ * covers (like {@code s3} for loading data from S3). Add the resources the spec needs (S3, Kafka, etc.) in
+ * {@link #addResources}, and the extensions and properties in {@link #configureCluster}.</li>
+ * <li>Write a {@code @Test} method that calls {@link #runSpec}, passing the settings of the resources to the spec
+ * through environment variables.</li>
+ * </ul>
+ * The tests run only with the {@code web-console-tests} profile, all of them or one class:
+ * <pre>
+ * mvn -pl embedded-tests verify -Pweb-console-tests
+ * mvn -pl embedded-tests verify -Pweb-console-tests -Dit.test=BatchIndexingWebConsoleTest
+ * </pre>
+ * With {@code -Dweb.console.keepAlive=true}, {@link #runSpec} doesn't run the spec. It prints the command to run it
+ * and waits, keeping the cluster up, so that the spec can be run (and debugged) from {@code web-console} against it.
+ * With {@code -Dweb.console.port}, the specs run against the console on that port (such as the dev server of
+ * {@code npm start}, which proxies to the Router on port 8888) rather than the console that the Router serves.
+ */
+@Tag("web-console")
+public abstract class WebConsoleTestBase extends EmbeddedClusterTestBase
+{
+ private static final Logger log = new Logger(WebConsoleTestBase.class);
+
+ private static final long SPEC_TIMEOUT_MINUTES = 15;
+ private static final Pattern NOT_FILE_NAME_CHARS = Pattern.compile("[^A-Za-z0-9.-]");
+
+ protected final EmbeddedCoordinator coordinator = new EmbeddedCoordinator();
+ protected final EmbeddedOverlord overlord = new EmbeddedOverlord();
+ protected final EmbeddedIndexer indexer = new EmbeddedIndexer();
+ protected final EmbeddedHistorical historical = new EmbeddedHistorical();
+ protected final EmbeddedBroker broker = new EmbeddedBroker();
+ protected final EmbeddedRouter router = new EmbeddedRouter();
+
+ @Override
+ protected final EmbeddedDruidCluster createCluster()
+ {
+ final EmbeddedDruidCluster cluster = EmbeddedDruidCluster.withEmbeddedDerbyAndZookeeper();
+ // Resources start in the order they are added, and must start before the Druid servers that use them
+ addResources(cluster);
+
+ // SleepModule has the sleep() function, for queries that run long enough to be canceled
+ cluster.addExtensions(SketchModule.class, HllSketchModule.class, DoublesSketchModule.class, SleepModule.class)
+ .addCommonProperty("druid.msq.dart.enabled", "true")
+ // Keep the reports of finished Dart queries, for the console to show them (as script/druid build does)
+ .addCommonProperty("druid.msq.dart.controller.maxRetainedReportCount", "100")
+ .addCommonProperty("druid.msq.dart.controller.maxRetainedReportDuration", "PT3600S")
+ .addCommonProperty("druid.sql.planner.enableSysQueriesTable", "true")
+ // The console waits for the cluster state to change, so make it change quickly
+ .addCommonProperty("druid.manager.segments.pollDuration", "PT1S")
+ .addServer(coordinator.addProperty("druid.coordinator.period", "PT1S"))
+ .addServer(overlord)
+ // Enough task slots for an MSQ query (a controller and a worker) next to an ingestion task
+ .addServer(indexer.addProperty("druid.worker.capacity", "4").setServerMemory(1_000_000_000L))
+ .addServer(historical)
+ .addServer(broker)
+ .addServer(router);
+
+ configureCluster(cluster);
+ return cluster;
+ }
+
+ /**
+ * Adds the resources (such as an S3 container) that the specs of this test need.
+ */
+ protected void addResources(EmbeddedDruidCluster cluster)
+ {
+ }
+
+ /**
+ * Adds the extensions and properties that the specs of this test need.
+ */
+ protected void configureCluster(EmbeddedDruidCluster cluster)
+ {
+ }
+
+ /**
+ * Runs the Playwright spec (a file name or a pattern of {@code web-console/e2e-tests}) against this cluster and
+ * fails if it fails.
+ */
+ protected void runSpec(String spec) throws Exception
+ {
+ runSpec(spec, Map.of());
+ }
+
+ /**
+ * Runs the Playwright spec (a file name or a pattern of {@code web-console/e2e-tests}) against this cluster and
+ * fails if it fails.
+ *
+ * @param env environment variables to pass to the spec, such as the settings of the resources
+ */
+ protected void runSpec(String spec, Map<String, String> env) throws Exception
+ {
+ final File webConsoleDir = webConsoleDir();
+ final Map<String, String> specEnv = new HashMap<>(env);
+ specEnv.put(
+ "DRUID_E2E_TEST_UNIFIED_CONSOLE_PORT",
+ System.getProperty("web.console.port", String.valueOf(router.bindings().selfNode().getPlaintextPort()))
+ );
+ // Each spec has its own output directory, as Playwright empties it on every run
+ final String outputDir = "test-results/" + NOT_FILE_NAME_CHARS.matcher(spec).replaceAll("_");
+ final List<String> command = List.of("npx", "playwright", "test", spec, "--output", outputDir);
+
+ if (Boolean.getBoolean("web.console.keepAlive")) {
+ keepAlive(webConsoleDir, specEnv, command);
+ return;
+ }
+
+ // The cluster is thrown away after this class, so the specs don't need to delete what they create. (A kept-alive
+ // cluster is reused for runs of the specs by hand, so they do.)
+ specEnv.put("DRUID_E2E_TEST_CLUSTER_IS_DISPOSABLE", "true");
+
+ log.info("Running spec[%s] in [%s] with env[%s].", spec, webConsoleDir, specEnv);
+ final ProcessBuilder processBuilder = new ProcessBuilder(command).directory(webConsoleDir)
+ .redirectErrorStream(true);
+ processBuilder.environment().putAll(specEnv);
+ final Process process = processBuilder.start();
+
+ // Log the output as it comes (so that a hung spec can be seen) and keep it for the failure message
+ final List<String> output = new ArrayList<>();
+ try (BufferedReader reader = new BufferedReader(
+ new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8)
+ )) {
+ String line;
+ while ((line = reader.readLine()) != null) {
Review Comment:
[P2] Enforce the spec timeout while consuming output
**Finding:** The runner drains `process.getInputStream()` to EOF before calling `waitFor` with `SPEC_TIMEOUT_MINUTES`. A hung Playwright process can keep stdout open, so the timeout branch is never reached and CI can hang until an external timeout.
**Suggestion:** Consume output asynchronously while waiting, then destroy the process and join or close the reader when the timeout expires.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "vogievetsky (via GitHub)" <gi...@apache.org>.
vogievetsky commented on code in PR #20544:
URL: https://github.com/apache/druid/pull/20544#discussion_r4285756985
##########
web-console/e2e-tests/component/load-data/sql-data-loader.ts:
##########
@@ -0,0 +1,87 @@
+/*
+ * 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.
+ */
+
+import type { Page } from '@playwright/test';
+
+import { clickButton, getLabeledInput, openView, setLabeledInput } from '../../util/playwright';
+import { showStep } from '../../util/steps';
+import { waitForTaskSlots } from '../workbench/overview';
+
+import type { DataConnector } from './data-connector';
+import { connectorCardTitle, fillConnector } from './data-connector';
+
+/**
+ * What to set at each step of the SQL data loader ("Load data" > "Batch - SQL"), which ingests with an MSQ task.
+ */
+export interface SqlDataLoaderConfig {
+ readonly connector: DataConnector;
+ /** Checks the input format that the data loader picked and the columns it parsed */
+ readonly validateParseData?: (parsed: { inputFormat: string; columns: string[] }) => void;
+ /** The new datasource to create */
+ readonly datasourceName: string;
+}
+
+/**
+ * Goes through each step of the SQL data loader, starts the ingestion and waits for it to finish. Throws if it fails.
+ */
+export async function loadDataWithSql(page: Page, config: SqlDataLoaderConfig): Promise<void> {
+ await openView(page, 'sql-data-loader');
+
+ // Select input type
+ await page
+ .locator('.input-source-step .bp6-card')
+ .filter({ has: page.locator('p', { hasText: connectorCardTitle(config.connector) }) })
+ .click();
+ await fillConnector(page, config.connector);
+ await showStep(page, `SQL data loader: ${connectorCardTitle(config.connector)}`);
+ await clickButton(page, 'Connect data');
+
+ // Parse
+ const parseDataTable = page.locator('.input-format-step .parse-data-table');
+ await parseDataTable.locator('.column-name').first().waitFor();
+ if (config.validateParseData) {
+ config.validateParseData({
+ inputFormat: await getLabeledInput(page, 'Input format'),
+ columns: await parseDataTable.locator('.column-name').allTextContents(),
+ });
+ }
+ await showStep(page, 'SQL data loader: Parse');
+ await clickButton(page.locator('.input-format-step .prev-next-bar'), 'Next');
+
+ // Schema, with the destination (a new datasource)
+ const schemaStep = page.locator('.schema-step');
+ await schemaStep.locator('.destination-button').click();
+ const destinationDialog = page.locator('.destination-dialog');
+ await clickButton(destinationDialog, 'New table');
+ await setLabeledInput(destinationDialog, 'New table name', config.datasourceName);
+ await showStep(page, 'SQL data loader: Destination');
+ await clickButton(destinationDialog, 'Save');
+ await destinationDialog.waitFor({ state: 'detached' });
+ await showStep(page, 'SQL data loader: Schema');
+ await clickButton(schemaStep.locator('.prev-next-bar'), 'Start loading data');
Review Comment:
That commit was a deliberate, temporary failure to see how an e2e failure shows in CI; it's reverted in a20cb856de.
##########
web-console/src/utils/table-filters/table-filter.ts:
##########
@@ -64,14 +96,18 @@ export class TableFilter {
return new TableFilter(filter.id, mode, value);
}
+ /**
+ * @param value the values to filter on, or a needle (as typed in a filter input) of values separated by `|` (see
+ * `splitNeedle`). To filter on one literal value, that may contain a `|`, pass it as `[value]`.
+ */
constructor(key: string, mode: FilterMode, value: string | string[]) {
this.key = key;
this.mode = mode;
- this.values = typeof value === 'string' ? value.split('|') : value;
+ this.values = typeof value === 'string' ? TableFilter.splitNeedle(value) : value;
Review Comment:
Good catch: fixed in 6be4e1fa31; the segment timeline's two links now pass the datasource as a literal value.
##########
web-console/e2e-tests/component/workbench/overview.ts:
##########
@@ -16,43 +16,181 @@
* limitations under the License.
*/
-import type * as playwright from 'playwright-chromium';
+import type { Locator, Page, Request } from '@playwright/test';
-import { clickButton, setQueryInput } from '../../util/playwright';
+import { expect } from '../../util/fixtures';
+import { clickButton, clickMenuItem, openView, setQueryInput } from '../../util/playwright';
+import { showStep } from '../../util/steps';
import { extractTable } from '../../util/table';
/**
- * Represents the workbench tab.
+ * How to run a query.
+ */
+export interface QueryOptions {
+ /** The engine to pick in the view's engine menu, by its name there (like "SQL (Dart)"), rather than "Auto" */
+ readonly engine?: string;
+}
+
+/**
+ * What the view sent to run a query.
+ */
+export interface SubmittedQuery {
+ /** The engine of the query's context, which the view sets for the engine picked (like "msq-dart") */
+ readonly engine?: string;
+ /** The ID that the view gives the query (SQL queries, natively or with Dart), to cancel it */
+ readonly sqlQueryId?: string;
+}
+
+/**
+ * Represents the Query view (the workbench).
*/
export class WorkbenchOverview {
- private readonly page: playwright.Page;
- private readonly baseUrl: string;
+ private readonly page: Page;
+
+ /** What the view sent to run the last query run (or started) */
+ lastSubmittedQuery: SubmittedQuery | undefined;
- constructor(page: playwright.Page, unifiedConsoleUrl: string) {
+ constructor(page: Page) {
this.page = page;
- this.baseUrl = unifiedConsoleUrl + '#workbench';
}
- async runQuery(query: string): Promise<string[][]> {
- await this.page.goto(this.baseUrl);
- await this.page.reload({ waitUntil: 'networkidle' });
+ /**
+ * Runs a query and returns its results (as text, one array of cells per row). Throws if the query fails.
+ */
+ async runQuery(query: string, options: QueryOptions = {}): Promise<string[][]> {
+ const results = this.page.locator('.result-table-pane');
+ await this.run(query, options, results);
+ await showStep(this.page, 'Query view: the query results');
+ return await extractTable(results);
+ }
+
+ /**
+ * Runs an ingestion query (INSERT or REPLACE, run as an MSQ task) and returns what the view says when it is done
+ * (like "39,244 rows inserted into ..."). Throws if the query fails.
+ */
+ async runIngestQuery(query: string, options: QueryOptions = {}): Promise<string> {
+ const ingestSuccess = this.page.locator('.ingest-success-pane');
+ await this.run(query, options, ingestSuccess);
+ await showStep(this.page, 'Query view: the data ingested');
+ return await ingestSuccess.innerText();
+ }
+
+ /**
+ * Starts a query, without waiting for it to finish, and returns what the view sent to run it.
+ */
+ async startQuery(query: string, options: QueryOptions = {}): Promise<SubmittedQuery> {
+ await this.submit(query, options);
+ await showStep(this.page, 'Query view: the query running');
+ return this.lastSubmittedQuery!;
+ }
+
+ private async run(query: string, options: QueryOptions, done: Locator): Promise<void> {
+ await this.submit(query, options);
+
+ const error = this.page.locator('.execution-error-pane');
+ await done.or(error).waitFor({ timeout: QUERY_TIMEOUT });
Review Comment:
Right, the view can show the last output while the next query runs; fixed in 09872dc940 by clearing the output before each query.
##########
embedded-tests/src/test/java/org/apache/druid/testing/embedded/console/WebConsoleTestBase.java:
##########
@@ -0,0 +1,231 @@
+/*
+ * 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.druid.testing.embedded.console;
+
+import org.apache.druid.guice.SleepModule;
+import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.java.util.common.logger.Logger;
+import org.apache.druid.query.aggregation.datasketches.hll.HllSketchModule;
+import org.apache.druid.query.aggregation.datasketches.quantiles.DoublesSketchModule;
+import org.apache.druid.query.aggregation.datasketches.theta.SketchModule;
+import org.apache.druid.testing.embedded.EmbeddedBroker;
+import org.apache.druid.testing.embedded.EmbeddedCoordinator;
+import org.apache.druid.testing.embedded.EmbeddedDruidCluster;
+import org.apache.druid.testing.embedded.EmbeddedHistorical;
+import org.apache.druid.testing.embedded.EmbeddedIndexer;
+import org.apache.druid.testing.embedded.EmbeddedOverlord;
+import org.apache.druid.testing.embedded.EmbeddedRouter;
+import org.apache.druid.testing.embedded.junit5.EmbeddedClusterTestBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Tag;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
+/**
+ * Base class for the web console end-to-end tests: a cluster with a Router (which serves the web console), on which
+ * the tests run the Playwright specs of {@code web-console/e2e-tests}.
+ * <p>
+ * Steps to write a test:
+ * <ul>
+ * <li>Write the Playwright spec in {@code web-console/e2e-tests}.</li>
+ * <li>Write a {@code *WebConsoleTest} class that extends this class, in the package of the functionality the spec
+ * covers (like {@code s3} for loading data from S3). Add the resources the spec needs (S3, Kafka, etc.) in
+ * {@link #addResources}, and the extensions and properties in {@link #configureCluster}.</li>
+ * <li>Write a {@code @Test} method that calls {@link #runSpec}, passing the settings of the resources to the spec
+ * through environment variables.</li>
+ * </ul>
+ * The tests run only with the {@code web-console-tests} profile, all of them or one class:
+ * <pre>
+ * mvn -pl embedded-tests verify -Pweb-console-tests
+ * mvn -pl embedded-tests verify -Pweb-console-tests -Dit.test=BatchIndexingWebConsoleTest
+ * </pre>
+ * With {@code -Dweb.console.keepAlive=true}, {@link #runSpec} doesn't run the spec. It prints the command to run it
+ * and waits, keeping the cluster up, so that the spec can be run (and debugged) from {@code web-console} against it.
+ * With {@code -Dweb.console.port}, the specs run against the console on that port (such as the dev server of
+ * {@code npm start}, which proxies to the Router on port 8888) rather than the console that the Router serves.
+ */
+@Tag("web-console")
+public abstract class WebConsoleTestBase extends EmbeddedClusterTestBase
+{
+ private static final Logger log = new Logger(WebConsoleTestBase.class);
+
+ private static final long SPEC_TIMEOUT_MINUTES = 15;
+ private static final Pattern NOT_FILE_NAME_CHARS = Pattern.compile("[^A-Za-z0-9.-]");
+
+ protected final EmbeddedCoordinator coordinator = new EmbeddedCoordinator();
+ protected final EmbeddedOverlord overlord = new EmbeddedOverlord();
+ protected final EmbeddedIndexer indexer = new EmbeddedIndexer();
+ protected final EmbeddedHistorical historical = new EmbeddedHistorical();
+ protected final EmbeddedBroker broker = new EmbeddedBroker();
+ protected final EmbeddedRouter router = new EmbeddedRouter();
+
+ @Override
+ protected final EmbeddedDruidCluster createCluster()
+ {
+ final EmbeddedDruidCluster cluster = EmbeddedDruidCluster.withEmbeddedDerbyAndZookeeper();
+ // Resources start in the order they are added, and must start before the Druid servers that use them
+ addResources(cluster);
+
+ // SleepModule has the sleep() function, for queries that run long enough to be canceled
+ cluster.addExtensions(SketchModule.class, HllSketchModule.class, DoublesSketchModule.class, SleepModule.class)
+ .addCommonProperty("druid.msq.dart.enabled", "true")
+ // Keep the reports of finished Dart queries, for the console to show them (as script/druid build does)
+ .addCommonProperty("druid.msq.dart.controller.maxRetainedReportCount", "100")
+ .addCommonProperty("druid.msq.dart.controller.maxRetainedReportDuration", "PT3600S")
+ .addCommonProperty("druid.sql.planner.enableSysQueriesTable", "true")
+ // The console waits for the cluster state to change, so make it change quickly
+ .addCommonProperty("druid.manager.segments.pollDuration", "PT1S")
+ .addServer(coordinator.addProperty("druid.coordinator.period", "PT1S"))
+ .addServer(overlord)
+ // Enough task slots for an MSQ query (a controller and a worker) next to an ingestion task
+ .addServer(indexer.addProperty("druid.worker.capacity", "4").setServerMemory(1_000_000_000L))
+ .addServer(historical)
+ .addServer(broker)
+ .addServer(router);
+
+ configureCluster(cluster);
+ return cluster;
+ }
+
+ /**
+ * Adds the resources (such as an S3 container) that the specs of this test need.
+ */
+ protected void addResources(EmbeddedDruidCluster cluster)
+ {
+ }
+
+ /**
+ * Adds the extensions and properties that the specs of this test need.
+ */
+ protected void configureCluster(EmbeddedDruidCluster cluster)
+ {
+ }
+
+ /**
+ * Runs the Playwright spec (a file name or a pattern of {@code web-console/e2e-tests}) against this cluster and
+ * fails if it fails.
+ */
+ protected void runSpec(String spec) throws Exception
+ {
+ runSpec(spec, Map.of());
+ }
+
+ /**
+ * Runs the Playwright spec (a file name or a pattern of {@code web-console/e2e-tests}) against this cluster and
+ * fails if it fails.
+ *
+ * @param env environment variables to pass to the spec, such as the settings of the resources
+ */
+ protected void runSpec(String spec, Map<String, String> env) throws Exception
+ {
+ final File webConsoleDir = webConsoleDir();
+ final Map<String, String> specEnv = new HashMap<>(env);
+ specEnv.put(
+ "DRUID_E2E_TEST_UNIFIED_CONSOLE_PORT",
+ System.getProperty("web.console.port", String.valueOf(router.bindings().selfNode().getPlaintextPort()))
+ );
+ // Each spec has its own output directory, as Playwright empties it on every run
+ final String outputDir = "test-results/" + NOT_FILE_NAME_CHARS.matcher(spec).replaceAll("_");
+ final List<String> command = List.of("npx", "playwright", "test", spec, "--output", outputDir);
+
+ if (Boolean.getBoolean("web.console.keepAlive")) {
+ keepAlive(webConsoleDir, specEnv, command);
+ return;
+ }
+
+ // The cluster is thrown away after this class, so the specs don't need to delete what they create. (A kept-alive
+ // cluster is reused for runs of the specs by hand, so they do.)
+ specEnv.put("DRUID_E2E_TEST_CLUSTER_IS_DISPOSABLE", "true");
+
+ log.info("Running spec[%s] in [%s] with env[%s].", spec, webConsoleDir, specEnv);
+ final ProcessBuilder processBuilder = new ProcessBuilder(command).directory(webConsoleDir)
+ .redirectErrorStream(true);
+ processBuilder.environment().putAll(specEnv);
+ final Process process = processBuilder.start();
+
+ // Log the output as it comes (so that a hung spec can be seen) and keep it for the failure message
+ final List<String> output = new ArrayList<>();
+ try (BufferedReader reader = new BufferedReader(
+ new InputStreamReader(process.getInputStream(), StandardCharsets.UTF_8)
+ )) {
+ String line;
+ while ((line = reader.readLine()) != null) {
Review Comment:
Fixed in d8a3312aae: the output is read on its own thread, and a timed-out spec's process tree is killed.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] feat: Web console: run the e2e tests on embedded clusters, modernize them, and cover more of the console (druid)
Posted by "vogievetsky (via GitHub)" <gi...@apache.org>.
vogievetsky commented on PR #20544:
URL: https://github.com/apache/druid/pull/20544#issuecomment-6272407172
@FrankChen021 re the `CollectAsyncResource` P1 from the latest review: this PR never touched `processing/`. #20504 landed on master after this branch's base, so comparing master's tip against the PR head made the fix look removed. I've merged master into the branch (5154498d23), so the diff no longer touches `processing/` and master's `CollectAsyncResource` is unchanged.
The CodeQL resource-leak alert on `KafkaWebConsoleTest` was already handled in the current code: the `FileInputStream` is its own try-with-resources resource. With that, I believe all the review findings are resolved.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org