-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix RTL e2e tests #26508
Fix RTL e2e tests #26508
Conversation
I'm happy to approve this as it seems good to me, and by all accounts it just needs approval from Travis above. |
Size Change: +151 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
Ok, so now all RTL tests fail on Travis but pass locally, which sounds like a failure to switch languages on a clean install. Maybe it has do to with loading the language packs? |
Anecdotally, tests have failed for me all week, on all 7 of my open PRs. So I think there's something else going on too. |
@@ -15,10 +15,12 @@ const ARABIC_TWO = '٢'; | |||
describe( 'RTL', () => { | |||
beforeEach( async () => { | |||
await createNewPost(); | |||
await page.evaluate( () => { | |||
document.querySelector( '.is-root-container' ).dir = 'rtl'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it seems reducing the scope of the layout shift is solving the issue. We're just testing rich text and writing flow here, so that's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is weird but I guess it's better than having wrong snapshots :)
Restores snapshots that were altered in #22213. I'm unsure what caused the failures, especially since it was a minor CSS change. After trial and error this method of changing the site language entirely seems much more robust.