Embeds as islands - #3963
Merged
Merged
Embeds as islands#3963
Embeds as islands#3963Conversation
oliverlloyd
requested review from
a team,
JamieB-gu and
marsavar
as code owners
February 10, 2022 17:20
|
Size Change: -6.4 kB (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
oliverlloyd
commented
Feb 11, 2022
Comment on lines
-38
to
-46
| describe('Ad slot Parity between DCR and Frontend for a long read', function () { | ||
| it(`It should check slots for a long article in DCR`, function () { | ||
| runLongReadTestFor(`Article?url=${longReadURL}`); | ||
| }); | ||
|
|
||
| it(`It should check slots for a long article in Frontend`, function () { | ||
| Cypress.config('baseUrl', ''); | ||
| runLongReadTestFor(`${longReadURL}?dcr=false`); | ||
| }); |
Contributor
Author
There was a problem hiding this comment.
I removed the Frontend test here because we no longer use Frontend!
DavidLawes
reviewed
Feb 11, 2022
| cy.scrollTo('bottom'); | ||
| cy.get(`[data-name="merchandising-high"]`).should('have.length', 1); | ||
| // Wait for hydration | ||
| cy.get('gu-island[name=MostViewedFooter]') |
Contributor
There was a problem hiding this comment.
I don't understand (help!) - how do embeds impact these tests?
Contributor
Author
There was a problem hiding this comment.
They don't directly. But they started failing and so I fixed them boy scout stylee
DavidLawes
reviewed
Feb 11, 2022
| 'Article?url=https://www.theguardian.com/sport/blog/2015/dec/02/the-joy-of-six-sports-radio-documentaries', | ||
| ); | ||
|
|
||
| cy.scrollTo(0, 4500); |
Contributor
There was a problem hiding this comment.
We're scrolling to a specific pixel position - will this always yield the next expected action (which I'm assuming scrolls something into view that then gets hydrated)?
Contributor
Author
There was a problem hiding this comment.
This is the expected behaviour, yes. Once our element is in view it becomes hydrated and we can continue the test verifying it does what we expect. The risk is content on this article moves around but that's unlikely (I hope!)
Contributor
|
🏝️ #3629 |
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
3 participants
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 does this change?
Embedsas islandsWhy?
Reduced js on the page