Fixing tt02 login for Cypress tests#4246
Conversation
…ing evaluateBefore functionality and replacing it with simpler goto functions
📝 WalkthroughWalkthroughThis PR refactors the e2e test infrastructure to centralize test fixture data, remove JavaScript injection-based navigation, and update authentication flows for improved cross-origin handling. A new Tenor fixture module provides shared user/organization test data; supporting infrastructure (auth, navigation, app instance defaults) is updated accordingly; and individual test files migrate to use the new patterns. ChangesTenor Fixture Centralization and Infrastructure Refactoring
Test Suite Updates to Use Tenor Fixtures
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…l because things were not loaded yet
… we waited a bit longer, as it would probably pass before the app was completely loaded.
… tests that don't do that. Overriding this in goto() so that frontend-test does not prompt for party most of the time.
…selectors plugin on a different origin
…lection() everywhere
…et of working Tenor users for an org
… check for #finishedLoading
…l page to not just trigger a hash-change
…r works. I suspect it's because the cypress runner no longer trusts the origin or something, but it's easier to just delete this test. I don't think this is something that will break at any point, as the test is simple enough and just asserts that clicking a button calls window.print().
…tching queries that really did finish
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/pageobjects/party-mocks.ts (1)
185-191:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the function comment to match the new strict behavior.
removeAllButKeepOrgnow throws when the org is missing, but the inline comment still says it falls back to the first org. This is now misleading.Suggested update
export function removeAllButKeepOrg(parties: IParty[], orgNumber: string): IParty[] { - // Keep one specific organisation (by org number) plus all persons. Pinning a specific org by - // number, instead of just taking the first one. This avoids depending on the order of the tt02 - // party list, which is not stable. Falls back to the first org when this specific org is not - // present, e.g. in docker/podman/localtest environments that have entirely different test data. + // Keep one specific organisation (by org number) plus all persons. Pinning a specific org by + // number, instead of just taking the first one, avoids depending on unstable party ordering. + // Throws if the target organisation is not present.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/pageobjects/party-mocks.ts` around lines 185 - 191, Update the inline comment above the org selection in removeAllButKeepOrg to reflect the current strict behavior: explain that the function pins a specific organisation by orgNumber and will throw an error if that org is not found (no longer falling back to the first organisation), and mention it still keeps all persons; reference the use of parties.find((party) => party.partyTypeName === PartyType.Organisation && party.orgNumber === orgNumber) and the subsequent throw when org is undefined so readers understand the behavior change.
🧹 Nitpick comments (1)
test/e2e/integration/signering-brukerstyrt/signing.ts (1)
30-30: ⚡ Quick winUse
lastNamefrom Tenor fixtures instead of splittingname.Parsing
name.split(' ')[1]is fragile if fixture names ever include middle names.TenorUser.lastNameis already available and safer.Suggested change
- cy.findByRole('textbox', { name: /navn/i }).type(Tenor.users.humanAndrefiolin.name.split(' ')[1]); + cy.findByRole('textbox', { name: /navn/i }).type(Tenor.users.humanAndrefiolin.lastName); - cy.findByRole('textbox', { name: /navn/i }).type(Tenor.users.standhaftigBjornunge.name.split(' ')[1]); + cy.findByRole('textbox', { name: /navn/i }).type(Tenor.users.standhaftigBjornunge.lastName); - cy.findByRole('textbox', { name: /etternavn/i }).type(Tenor.users.varsomDiameter.name.split(' ')[1]); + cy.findByRole('textbox', { name: /etternavn/i }).type(Tenor.users.varsomDiameter.lastName);Also applies to: 46-46, 79-79
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/integration/signering-brukerstyrt/signing.ts` at line 30, Replace fragile name splitting with the fixture's lastName field: wherever the test uses Tenor.users.humanAndrefiolin.name.split(' ')[1] (and the other similar occurrences in this file), use Tenor.users.humanAndrefiolin.lastName instead; update the cy.findByRole('textbox', { name: /navn/i }).type(...) calls to pass the lastName value so tests don't break if a fixture has middle names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@test/e2e/pageobjects/party-mocks.ts`:
- Around line 185-191: Update the inline comment above the org selection in
removeAllButKeepOrg to reflect the current strict behavior: explain that the
function pins a specific organisation by orgNumber and will throw an error if
that org is not found (no longer falling back to the first organisation), and
mention it still keeps all persons; reference the use of parties.find((party) =>
party.partyTypeName === PartyType.Organisation && party.orgNumber === orgNumber)
and the subsequent throw when org is undefined so readers understand the
behavior change.
---
Nitpick comments:
In `@test/e2e/integration/signering-brukerstyrt/signing.ts`:
- Line 30: Replace fragile name splitting with the fixture's lastName field:
wherever the test uses Tenor.users.humanAndrefiolin.name.split(' ')[1] (and the
other similar occurrences in this file), use
Tenor.users.humanAndrefiolin.lastName instead; update the
cy.findByRole('textbox', { name: /navn/i }).type(...) calls to pass the lastName
value so tests don't break if a fixture has middle names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a51c9046-2a4d-40ad-b17c-a9269e914bb2
📒 Files selected for processing (27)
test/README.mdtest/e2e/config/tt02.jsontest/e2e/integration/anonymous-stateless-app/anonymous.tstest/e2e/integration/anonymous-stateless-app/auto-save-behavior.tstest/e2e/integration/anonymous-stateless-app/options.tstest/e2e/integration/anonymous-stateless-app/validation.tstest/e2e/integration/frontend-test/accordion.tstest/e2e/integration/frontend-test/auto-save-behavior.tstest/e2e/integration/frontend-test/instantiation.tstest/e2e/integration/frontend-test/party-selection.tstest/e2e/integration/frontend-test/pdf.tstest/e2e/integration/frontend-test/prefill.tstest/e2e/integration/frontend-test/print-button.tstest/e2e/integration/frontend-test/self-identified-user.tstest/e2e/integration/signering-brukerstyrt/signing.tstest/e2e/integration/stateless-app/party-selection.tstest/e2e/integration/stateless-app/receipt.tstest/e2e/integration/stateless-app/stateless.tstest/e2e/pageobjects/app-frontend.tstest/e2e/pageobjects/party-mocks.tstest/e2e/support/apps/signing-test/signing-login.tstest/e2e/support/auth.tstest/e2e/support/custom.tstest/e2e/support/global.tstest/e2e/support/navigation.tstest/e2e/support/start-app-instance.tstest/e2e/support/users.ts
💤 Files with no reviewable changes (1)
- test/e2e/integration/frontend-test/print-button.ts
Description
login.htmlfiles in apps so tests can establish the app origin before redirecting through login (this fixes lots of small problems that crept up after switching to Tenor login).evaluateBeforepre-app JavaScript shortcut and rewrotecy.goto()to navigate through the app UI/process flow, because the previous shortcut no longer worked reliably with the new login/origin setup.cy.preventPartySelection()and updated party-selection tests to explicitly opt in/out of party prompting, avoiding accidental prompts in tests that assume direct app entry.login.htmlbefore loading PDF URLs, avoiding app-frontend treating PDF navigation as only a hash change (this lead to flakiness in several pdf tests).window.print()no longer works reliably with the current Cypress/origin behavior, and the test only covered the browser print call (so it was not all that useful).subform-testto the current Altinn Studio URL (one app mistakenly pointed to dev.altinn.studio when it was on altinn.studio).Related Issue(s)
Verification/QA
kind/*andbackport*label to this PR for proper release notes groupingSummary by CodeRabbit
Tests
Chores