Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cypress tests transliteration scriptSwitching #12500

Merged
merged 39 commits into from
Mar 24, 2025

Conversation

MeriemMechri
Copy link
Contributor

@MeriemMechri MeriemMechri commented Mar 11, 2025

Resolves JIRA [WORLDSERVICE-106]

Overall changes

  • Unskipped and fixed the flakiness for transliteration/script switching.
  • Updated fixture data for the following services: Uzbek, Serbia, zohongwen.
  • Removed onDemandAudio tests.

Testing

  • PR checks passing

  • Smoke Tests passing on test: CYPRESS_APP_ENV=test yarn cypress:interactive

  • Smoke Tests passing on live: CYPRESS_APP_ENV=live yarn cypress:interactive

  • Non-Smoke Tests (Scheduled E2Es) passing on test: CYPRESS_SMOKE=false CYPRESS_APP_ENV=test yarn cypress:interactive

  • Non-Smoke Tests (Scheduled E2Es) passing on live: CYPRESS_SMOKE=false CYPRESS_APP_ENV=live yarn cypress:interactive

Helpful Links

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

@MeriemMechri MeriemMechri changed the title (DRAFT) Cypress-tests-transliteration-script-switching Cypress-tests-transliteration-script-switching Mar 13, 2025
@karinathomasbbc
Copy link
Contributor

We can fix the broken integration tests by running yarn test:integration:updatesnapshots (which have failed because we've updated the fixture data used by some of these tests).

@LilyL0u
Copy link
Contributor

LilyL0u commented Mar 13, 2025

When I run the tests against test, one of the tests has a problem where when it is looking for the first <a> on the page to visit the next page in the user journey, it is including the cookie banner <a> in it, because for some reason the cookie banner pops up on this page! This means the next test then fails as it goes to https://www.bbc.co.uk/userinfo which doesn't have the elements we are expecting on it.

Screenshot 2025-03-13 at 13 57 10

So we need to work out a fix for this for it to not fail in the test e2es.

@LilyL0u
Copy link
Contributor

LilyL0u commented Mar 13, 2025

When I ran the tests locally and on test I noticed that (like we were talking about this morning) the article pages clicked on locally always go to an error page, and then the script switch tests run on the error page. They are then not failing because the test then just checks the script switch works, and it does work on error pages, but it means locally and on test it is not testing article pages work with the script switch, just a lot of error page tests!

However, on live it does click valid article pages, and so the tests do run on article pages on live. Do we mind that article pages are not having the script switch checked on local and test, as long as they are covered on live?

@LilyL0u
Copy link
Contributor

LilyL0u commented Mar 13, 2025

When I run the tests against test, one of the tests has a problem where when it is looking for the first <a> on the page to visit the next page in the user journey, it is including the cookie banner <a> in it, because for some reason the cookie banner pops up on this page! This means the next test then fails as it goes to https://www.bbc.co.uk/userinfo which doesn't have the elements we are expecting on it.
Screenshot 2025-03-13 at 13 57 10

So we need to work out a fix for this for it to not fail in the test e2es.

I also ran against live and had the same issue BUT I realised this only happens the first time I run the tests. The next time I run the tests it doesn't happen (probably related to the cookie banner behaviour beign different after first visit). So the question is will this accidential cookie banner click happen in the AWS E2E runs, or not? Does this make the test flakey or does it never do this 🤔

@MeriemMechri
Copy link
Contributor Author

@karinathomasbbc when I ran this command yarn test:integration:updatesnapshots I got the following failures. Is there a pagetype that I need to add?

image

@karinathomasbbc
Copy link
Contributor

@karinathomasbbc when I ran this command yarn test:integration:updatesnapshots I got the following failures. Is there a pagetype that I need to add?
image

Oh dear, not sure why that happened - let me see if it happens to me? Make sure you're not already running yarn dev for Simorgh / NextJS. Did it manage to generate snapshots for Simorgh page types? That might just be enough, since no Next JS pages are impacted by this PR.

@MeriemMechri MeriemMechri changed the title Cypress-tests-transliteration-script-switching Cypress tests transliteration scriptSwitching Mar 17, 2025
@MeriemMechri MeriemMechri force-pushed the Cypress-tests-transliteration-script-switching branch from a723ff0 to 4628f21 Compare March 20, 2025 09:30
Copy link
Contributor

@amoore108 amoore108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach looks good to me. Thanks for tackling this 👍

@MeriemMechri MeriemMechri force-pushed the Cypress-tests-transliteration-script-switching branch from 51a6fba to d64296f Compare March 20, 2025 13:42
@MeriemMechri MeriemMechri force-pushed the Cypress-tests-transliteration-script-switching branch from e3fe4c4 to 78cc54c Compare March 20, 2025 19:54
@amoore108 amoore108 merged commit 76dc1ce into latest Mar 24, 2025
11 checks passed
@amoore108 amoore108 deleted the Cypress-tests-transliteration-script-switching branch March 24, 2025 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants