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: [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]
