fix: [DHIS2-20898] re-enable and de-flake BreakingTheGlass Cypress test - #4694
Open
karolinelien wants to merge 7 commits into
Open
fix: [DHIS2-20898] re-enable and de-flake BreakingTheGlass Cypress test#4694karolinelien wants to merge 7 commits into
karolinelien wants to merge 7 commits into
Conversation
The scenario logged out and logged back in as a second user by driving
the raw login form (cy.visit('/') + fill + submit). On the current
standalone/prod Cypress build, an unauthenticated visit hard-redirects
to the backend's own hosted login page on a different origin, which
Cypress can't interact with without cy.origin() - causing a consistent
cross-origin failure, not just occasional flakiness.
Switch the user-switch steps to cy.session()/cy.loginByApi(), the same
pattern every other test in the suite already uses, sidestepping the
login form entirely since this scenario doesn't need to test it.
AI Assisted
The tracker2 credential is a non-production test-instance account, not a secret - same value that was already inline in this file before the loginByApi refactor, just in a syntactic shape Sonar's S2068 heuristic now matches. AI Assisted
Mirror the house session pattern in cypress/support/tagUtils/login.js: the switched-user session had no validate callback, so a stale cached session or a login that silently kept the previous user would surface as a confusing missing-widget failure several steps later instead of failing at the switch.
The scenario creates a tei enrolled in Child programme at Ngelehun and only cleared it on the way in, so it outlived the run. That is the same working list TrackerWorkingListsUser asserts on by row position, and the leftover row shifted every assertion there - visible as 6 failures on 'Filona'/'Johnny'/'Donald' across every instance version, while master's shard 5 was green. Sorting ascendingly by first name put 'Breaking' ahead of 'Donald', which pinned it. Adds a tag-scoped After hook following the house pattern, re-authenticating as the default user first because the scenario ends as the search-scope-only one.
simonadomnisoru
approved these changes
Aug 25, 2026
henrikmv
approved these changes
Aug 25, 2026
The hard-coded-credential smell is handled as a false positive in SonarQube instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…akingtheglass-flaky-test
|
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
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.

What
Re-enables the
BreakingTheGlass.featurescenario (User with search scope access tries to access an enrollment in a protected program), disabled via@skipsince March 2024, by fixing the actual causes of the failure rather than leaving it disabled (https://dhis2.atlassian.net/browse/DHIS2-20898).Why
1. The user switch drove the login form directly
The scenario switched users mid-test via the raw login form (
cy.visit('/')+ fill + submit) instead of thecy.session()/cy.loginByApi()pattern every other test in the suite uses. On the current standalone/prod Cypress build, an unauthenticated visit hard-redirects to the backend's own hosted login page on a different origin, which Cypress can't interact with withoutcy.origin():This looks like the same underlying regression as https://dhis2.atlassian.net/browse/DHIS2-20638 (also disabled, also driving the login form directly, its own code comment blames "login form is not working for v42 currently") — both likely trace back to the App Platform v12 upgrade (https://dhis2.atlassian.net/browse/DHIS2-18801). Unlike that ticket, this scenario doesn't need to test the login form UI itself, so switching the user-switch step to
cy.loginByApi()sidesteps the problem entirely instead of needing acy.origin()-based fix.The switched-user session also gets a
validatecallback asserting/api/meis the expected user, mirroring the house pattern incypress/support/tagUtils/login.js. Without it, a stale cached session — or a login that silently kept the previous user — surfaces as a confusing missing-widget failure several steps later instead of failing at the switch, which matters because "the second user is genuinely a different, less-privileged user" is the whole premise of the scenario.2. The scenario leaked its tei into another spec's assertions
Re-enabling the scenario alone turned
TrackerWorkingListsUser.featurered — 6 failures onExpected to find content: 'Filona'/'Johnny'/'Donald' within the element: <tr>, on every instance version, while master's shard 5 was green.Cause: this scenario creates a tei enrolled in Child programme at Ngelehun and only cleared it on the way in (
Given the tei created by this test is cleared from the database), so the tei outlived the run. That is the same working listTrackerWorkingListsUserasserts on by row position — a positional array of first names matched row-by-row — so one extra row shifted every pagination and ordering assertion. The clincher: "Show teis ordered ascendingly by first name" expectedDonald, and the leaked tei's first name isBreaking, which sorts ahead of it.Fixed by adding a tag-scoped
Afterhook following the house pattern (@with-...-cleanup, as inWidgetProfile,TrackerBulkActions,EditEventPageForm), re-authenticating as the default user first because the scenario ends logged in as the search-scope-only one, which cannot delete the tei.Verification
Local, before the session fix: 4/5 runs failed with the cross-origin error above. After: 7/8 clean passes, zero recurrences.
In CI,
BreakingTheGlass.featureitself passes on all four targets — dev (2.44-SNAPSHOT), 2.41, 2.42 and 2.43 — confirming thetracker2user still has the search-scope-but-not-capture-scope access the scenario depends on. The spec runs in shard 3; the shard-5TrackerWorkingListsUserfailures described above are what theAfterhook addresses.Unrelated to this PR: the 2.42 instance is currently unhealthy — all five of its shards fail with
cy.request() failed on:across specs this branch does not touch.AI Assisted