test: add an end-to-end browser suite for the UI - #762
Open
blaipr wants to merge 4 commits into
Open
Conversation
The selector in the job tab bar always read "Workflow Job 1/X", whatever task you were on, and picking another task did nothing. Closes ctrliq#742. Three separate defects, only the first of which was in the selector's own counter: - The position was the literal 1, and the list it counted excluded the job on screen, so a four task workflow read 1/3 on every one of its tasks. Count every node that ran a job, the current one included, and report where in that list the job on screen sits. - Picking a task did nothing because the selector is registered in Job.js as a tab with no link, so that it sits in the tab bar. The click bubbles to the PatternFly tab, and RoutedTabs called navigate(undefined) for it, landing back on the current url and undoing the navigation the selector had just performed. RoutedTabs now leaves link-less tabs alone. - RoutedTabs passed href={!tab.hasstyle && `#${tab.link}`}, which is the boolean false for exactly that tab. React rejects false as an href. Two more found while testing the above: - With a status filter applied, the toggle rendered a Label with a close button inside the toggle's own button. Nested buttons are invalid, and that close button was unclickable for the same bubbling reason. The filter is cleared by picking the same status again. - The menu seeded useState from the first render, so after moving between jobs it still offered the list belonging to the page you came from. The list is derived now. The old component test mocked out the component under test and asserted against the mock, which is why none of this was caught. It is replaced by twelve tests against the real component, plus two on RoutedTabs covering tabs that host a control.
The selector's label changed, so its message id changed with it, and a
production build renders an id it has no catalog entry for. The tab read
"-t1kzb" rather than "Workflow Job 2/4" until the catalogs were extracted
again. The jest suite does not see this, and neither does the development
server, which compiles catalogs on the way up; only a built UI shows it.
The two new messages are named rather than positional,
"Workflow Job {currentPosition}/{total}" instead of leaving translators a
bare {0}.
Extraction also picks up three messages that were already in the source
and missing from the catalogs, and refreshes the source line references
throughout, which is what makes the diff large. The Spanish, French and
other translations of the old "Workflow Job 1/{0}" are dropped, as they
have to be: that string no longer exists.
The jest suite runs in jsdom, which does not reproduce event bubbling through the DOM, focus, or anything rendered through a portal. ctrliq#742 is the worked example: a jest test asserting that picking a job navigates passed against code where the click did nothing in every real browser, because the click bubbled out of the selector into the tab hosting it. Nothing in the existing suite could have caught that, and more unit tests would not change it. Ten specs over the flows worth protecting: signing in, job tabs, and the workflow job selector. They assert on the url and on rendered text rather than on internals, and fail on console errors the page is not expected to log, which is how two of the three defects in ctrliq#742 first showed themselves. The fixtures are built from system job templates. Every instance has them, they need no project, inventory, credential or network access, and they finish in seconds, so a multi-node workflow job can be seeded with nothing that can be slow or absent. It also means the specs navigate under /jobs/management/, exercising a url segment other than the default. The suite is its own package under awx/ui/e2e so that installing the UI does not pull a browser driver in, and it stays out of npm test so the unit suite stays fast. The CI job restarts the web process after building the UI. The UI is built after the server is already serving and Django caches the index template naming the bundle, so without that step the tests would run against whatever the image shipped rather than against the pull request.
This was referenced Aug 26, 2026
Every other check in this workflow is lowercase and hyphenated, api-test through ui-test-general and dev-env, because they come from the matrix or from the job key. A title-cased name stood out, and the display name is what a required status check rule is written against, so it is worth settling deliberately.
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
1 participant
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.
SUMMARY
Adds an end-to-end browser suite for the UI and runs it as a CI job. Related #761.
Stacked on #760. This branch carries that fix as well, because the suite's regression specs are cover for exactly that bug and would fail without it. Once #760 merges this reduces to
awx/ui/e2e/, the CI job, and one line in the eslint config.Why a browser and not more jest
The jest suite is large and healthy, 552 files and around 2950 tests at roughly 75% of statements. It runs in jsdom, which does not reproduce event bubbling through the DOM, focus, or anything rendered through a portal.
#742 is the worked example. A jest test asserting "picking a job navigates" passed against code where the click did nothing in every real browser: the click bubbled out of the selector into the tab that hosts it, and
RoutedTabscallednavigate(undefined), landing back on the current url. No amount of unit testing reaches that.Coverage was not the gap either. The file at the centre of #742 had a test all along, one that mocked out the component under test and asserted against the mock. It passed for as long as the bug existed.
What it covers
Ten specs, deliberately few, over flows where a break is likely and expensive:
login.spec.jstabs.spec.jsRoutedTabs, which around forty screens sharejob-output.spec.jsThey assert on the url and on rendered text rather than on internals, and fail on console errors the page is not expected to log. Two of the three defects in #742 first announced themselves that way.
The fixtures
seed.jsruns once before the specs and builds a workflow from system job templates. Every instance has them, they need no project, inventory, credential or network access, and they finish in seconds, so a multi-node workflow job can be seeded without depending on anything slow or absent. It also means the specs navigate under/jobs/management/, exercising a url segment other than the default one, which is where a missing entry in the type-to-segment map would hide.The CI job
A job keyed
e2ein the existing.github/workflows/ci.yml, reporting as the checkui-e2e. Lowercase and hyphenated to sit withapi-test,ui-lintanddev-envrather than stand out, since the display name is what a required status check rule would be written against.It runs on pull requests, like
dev-env, and reuses the samerun_awx_develaction withbuild-ui: trueso it drives the built UI rather than a development server.One step needs explaining. The action builds the UI after the web process is already serving, and Django caches the index template that names the bundle, so the job restarts
awx-uwsgiand waits for the API to come back. Without it the tests run against whatever the image shipped rather than against the pull request. I found this the hard way: the suite reported the bug as unfixed against a UI that had the fix compiled into it.The report and traces upload as an artifact on failure.
ISSUE TYPE
COMPONENT NAME
ASCENDER VERSION
ADDITIONAL INFORMATION
Verified both ways against a running environment:
The five that fail are precisely the #742 regression specs, which is the demonstration that the suite earns its place.
It also caught something the jest suite and the development server both miss: changing the selector's label changed its lingui message id, and a production build renders an id with no catalog entry. The tab read
-t1kzbinstead ofWorkflow Job 2/4until the catalogs were extracted again. That fix is the second commit on #760.Running it locally needs an environment that is already up:
awx/ui/e2e/README.mdcovers the configuration and how to add a spec.It runs on pull requests, alongside
dev-env, because that is where it catches things before they land.