WSTEAMA 1276 - onDemand TV - Remove tests (Unit, integration and E2E) against AMP versions of these pages

[holchris]

Resolves JIRA 1276

Overall changes

We want to remove the morph media components for all on demand TV pages. These pages currently have AMP versions which require an iFrame integration for media players.

Code changes

• Remove isAmp from onDemandTV e2e
• Remove integration ampTests for onDemandTV

Testing

[amoore108]

Do we need to update the OnDemandTV page unit tests to remove references to AMP? https://github.com/bbc/simorgh/blob/eb13e8820661c929cd4cb7c3b2ebb0e0d65cc6a4/src/app/pages/OnDemandTvPage/index.test.jsx

[LilyL0u]

As @amoore108 pointed out we can remove the isAmp arguments into renderPage in each test, and also remove the unit tests specific to amp e.g https://github.com/bbc/simorgh/blob/eb13e8820661c929cd4cb7c3b2ebb0e0d65cc6a4/src/app/pages/OnDemandTvPage/index.test.jsx#L222

Also, I don't think we came to a decision about this in refinement, but we can also take the tests out of testsForCanonicalOnly, put them into tests.js, and update the index.cy.js file so we don't have testForCanonicalOnly and crossPlatformTests both run. We can also rename crossPlatformTests to something else as we aren't referring to both canonical and amp anymore https://github.com/bbc/simorgh/blob/1497df2e0b913c3a33a86cc06fae72f179f82bee/cypress/e2e/pages/onDemandTV/index.cy.js#L5. (a default export from tests.js in that folder has been named crossPlatformTests but can be named anything).

[LilyL0u]

Do we also want to remove .amp from the route regex and change the tests to have .amp urls as invalid?

https://github.com/bbc/simorgh/blob/796c4d0a400ee8e460aa66df272223a05bd79a88/src/app/routes/utils/regex/index.test.js#L361-L385

https://github.com/bbc/simorgh/blob/796c4d0a400ee8e460aa66df272223a05bd79a88/src/app/routes/utils/regex/utils/index.js#L152-L155

[karinathomasbbc]

Chucking on the DO NOT MERGE label, as I think we want to wait until the Belfrage routing PR has merged...

Although worth asking for a second opinion: @andrewscfc / @amoore108 / @alex-magana

Is it better for a route to 404 in Belfrage, OR in Simorgh? From the code changes in the PR, we're removing support for the .amp route for OD TV pages. However, we are also working on a Belfrage PR which will 404 the .amp route. I am not sure which one should come first (or whether it even matters?) but my gut feeling is that it's better to 404 at a higher level in the chain...?

[amoore108]

Chucking on the DO NOT MERGE label, as I think we want to wait until the Belfrage routing PR has merged...

Although worth asking for a second opinion: @andrewscfc / @amoore108 / @alex-magana

Is it better for a route to 404 in Belfrage, OR in Simorgh? From the code changes in the PR, we're removing support for the .amp route for OD TV pages. However, we are also working on a Belfrage PR which will 404 the .amp route. I am not sure which one should come first (or whether it even matters?) but my gut feeling is that it's better to 404 at a higher level in the chain...?

Yea I'd personally advise 404ing in Belfrage.

Tbh, I feel like the catch-all in Belfrage will catch the missing route and send it to Mozart, which should 404 with its static 404 page and not reach Simorgh, but at least with Belfrage sending the 404 we'll be able to show translated 404 pages.