Add Playwright UI tests for the JWT build feature editor#118
Merged
Conversation
Spins up a real TeamCity 2025.11 server via Testcontainers, creates a
project and build type via the REST API, then drives the build feature
editor in Chromium and asserts that form state round-trips correctly.
Covers eleven scenarios: default-uncheckedness of subject dimensions,
persistence of each editable field (subject dimensions, algorithm,
TTL, audience) across save/reopen, the live "Resulting sub claim"
preview reflecting checkbox toggles, the empty-dimensions sentinel
("none") persisting rather than silently collapsing back to the
default, and the two server-side validation errors (invalid TTL,
unknown subject dimension) blocking save.
Adds Playwright 1.44 as a test-scope dependency and an
exec-maven-plugin execution in pre-integration-test phase that runs
`com.microsoft.playwright.CLI install chromium` in a forked JVM (the
CLI calls System.exit() so it can't share the test JVM).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous maven:3.9-amazoncorretto-21 tag is built on Amazon Linux 2, which ships glibc 2.26. Playwright 1.44 bundles a Node binary that requires glibc 2.27/2.28, so the playwright-install-chromium Maven goal added in the previous commit fails to start the bundled Node before the tests can even run. The -al2023 variant has the same JDK distribution and Maven version on Amazon Linux 2023, which ships glibc 2.34 — enough for the bundled Node to load. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous bump to Amazon Linux 2023 cleared the glibc issue and let the playwright-install-chromium goal complete, but Chromium itself refuses to launch on AL2023 because the system libraries it links against at runtime (libnss3, libatk, libgbm, libxkbcommon, etc) are not present. Playwright's install-deps helper shells out to apt-get, which does not exist on AL2023, and Playwright does not publish a dnf equivalent or AL2023 package mapping. Switch the integration-tests image to maven:3.9-eclipse-temurin-21 (Debian bookworm, glibc 2.36) and add --with-deps to the playwright install argument list. That single flag tells Playwright to apt-get install the dependency set it knows Chromium needs, in addition to downloading the browser binary itself. Two CI failures in a row caused by image-distro choice — the underlying issue was that Playwright is built and tested against Debian/Ubuntu, and patching around a non-supported distro keeps producing new failure modes (first glibc, then runtime .so dependencies). Moving to a base image Playwright already targets removes that whole class of problem. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
XML forbids `--` anywhere inside a comment body except at the opening and closing boundaries. The prose in the playwright-install-chromium comment referred to the flag literally as `--with-deps`, which the Maven XML parser rejected with: [FATAL] Non-parseable POM ... in comment after two dashes (--) next character must be > not w The `<argument>--with-deps</argument>` element below the comment is fine — element content has no such restriction. Only the comment body needed adjusting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Playwright 1.44.0 was released before Ubuntu 24.04 (noble) shipped, so its `install --with-deps` helper hardcodes `libasound2` in the apt-get package list. Noble renamed that package to `libasound2t64` as part of the 64-bit time_t transition, and the install fails with: E: Package 'libasound2' has no installation candidate Failed to install browsers Error: Installation process exited with code: 100 The previous image bump landed on `maven:3.9-eclipse-temurin-21`, which turns out to be Ubuntu noble in current rolling releases (not Debian bookworm as the prior commit message anticipated — the eclipse-temurin base moved to noble during the 24.04 LTS rollout). Pinning the image to jammy would fix this for our specific case, but the underlying cause is the year-old Playwright pin. Bumping to 1.52.0 (current latest) gives us noble support in install-deps plus eight minor versions of bug fixes. The test only uses core Browser/Page/Locator APIs which are stable across this range; local `mvn test-compile -pl integration-tests` succeeds with no source changes needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
matt-richardson
commented
May 18, 2026
|
|
||
| integration-tests: | ||
| image: maven:3.9-amazoncorretto-21 | ||
| image: maven:3.9-eclipse-temurin-21 |
Contributor
Author
There was a problem hiding this comment.
Needed to switch this around, to get a base image that playwright could run on
mattburnell
approved these changes
May 20, 2026
mattburnell
left a comment
mattburnell
left a comment
There was a problem hiding this comment.
This is very nice! I like the break-up, the lifecycle of things, and the tests themselves are very easy to read. There are a couple of places I think the last vestiges of repetition could be removed, but overall this is really nicely factored.
| </configuration> | ||
| </plugin> | ||
| <!-- | ||
| Downloads Chromium browser binaries and the system shared libraries |
There was a problem hiding this comment.
While not inaccurate, it might be worth mentioning in the comment that Playwright itself is used to download / set up these things. It's definitely covered in the code / definition below, but the wording here made me think "wait, I thought Playwright could manage all this for you..." and then I saw that it's just calling it with --with-deps chromium and thought "ahh, good".
|
|
||
| @Test | ||
| void checkingDimensionPersistsAfterSave() throws Exception { | ||
| // Uses trigger_type because the branch checkbox is only rendered when the build |
There was a problem hiding this comment.
I think this comment offers more confusion than enlightenment; is it just a Claude vestige?
| saveBuildFeature(page); | ||
| } | ||
|
|
||
| // Phase 2: uncheck every checkbox, verify empty subject_dimensions is stored |
There was a problem hiding this comment.
It's not super-harmful repetition in this case by any means, but this could be a HoF (e.g. withAllDimensionCheckboxes) that takes an action on a checkbox, and is just called with x => x.check() and x => x.uncheck()
|
|
||
| @Test | ||
| void algorithmSelectionPersistsAfterSave() throws Exception { | ||
| try (final var context = newLoggedInContext()) { |
There was a problem hiding this comment.
Building on my comment above about a HoF... they could be stacked. Many of these tests have the same 5-line block that's "build context, navigate to page, , call save".
The previous wording read like the Maven plugin itself was downloading Chromium and apt-installing its deps, which left a reviewer wondering "wait, doesn't Playwright handle that?" Reword to lead with "Invokes Playwright's own CLI" so the relationship is clear (and avoid literal '--' inside an XML comment, which is illegal). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every test was spelled out as the same 5-line "open context, new page, navigate to editor, do thing, [save]" block. Extract two stacked helpers so each test reads as the action that matters: * inFeatureEditor(Consumer<Page>) — opens a fresh logged-in editor and runs the action; used by read-only and validation-error tests. * editFeature(Consumer<Page>) — same, plus clicks Save and waits for the modal to close; used by the round-trip persistence tests. A third helper, forEachDimensionCheckbox(page, action), collapses the two identical check/uncheck loops in uncheckingAllDimensionsSaves... to forEachDimensionCheckbox(page, Locator::check) / Locator::uncheck. Also drops a misleading comment on checkingDimensionPersistsAfterSave that explained why it tested trigger_type — picking any single dimension is fine; the comment was conversational noise. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
The JWT build feature editor has accumulated enough custom JavaScript — the subject-dimensions checkbox group, the live "Resulting
subclaim" preview, thenonesentinel for the empty-dimensions case, the algorithm/TTL/audience fields, server-side validation errors — that regressions in the form's persistence behaviour are easy to introduce and hard to spot without actually clicking through it. Until now there was no automated coverage of the editor as it renders in a browser; everything was either a JSP unit test or a Selenium-free Testcontainers IT that talked HTTP to the REST API.Details
The kinds of regressions we've hit historically:
nonesentinel fix in Make sub customisable rather than claims #112 and reviewers' "Unticking the 'trigger' claim doesn't actually stick on save" item).subpreview drifting out of sync with the persisted form.These all live above the JSP/JavaScript boundary, so JSP-only unit tests can't catch them.
Results
New integration test class
BuildFeatureUIITunderintegration-tests/, run by Failsafe in the existingmvn verifyphase. It:jetbrains/teamcity-server:2025.11via Testcontainers with the plugin zip mounted.Eleven scenarios are covered:
allDimensionsUncheckedByDefaultInUisub.checkingDimensionPersistsAfterSaveuncheckingAllDimensionsSavesEmptyPropertyValuenonesentinel persists; doesn't collapse to default.algorithmSelectionPersistsAfterSavettlMinutesPersistsAfterSaveaudiencePersistsAfterSaveliveSubjectPreviewIsMinimalByDefaultsubpreview reflects the default config.liveSubjectPreviewReflectsCheckboxTogglesubpreview updates as checkboxes toggle.savedSubjectDimensionsArePreCheckedOnReopeninvalidTtlShowsValidationErrorAndDoesNotSaveunknownSubjectDimensionShowsValidationErrorAndDoesNotSaveBuild-side changes in
integration-tests/pom.xml:com.microsoft.playwright:playwright:1.44.0as a test-scope dependency.exec-maven-pluginexecution inpre-integration-testthat runscom.microsoft.playwright.CLI install chromiumin a forked JVM. The fork is required becauseCLI.main()callsSystem.exit()when finished, which would kill Surefire/Failsafe if it ran in the build JVM.Screenshots
N/A — this PR adds tests against existing UI; no UI changes of its own.