Fix Cypress path checks to allow sub-directory#4945
Fix Cypress path checks to allow sub-directory#4945gziolo merged 2 commits intoWordPress:masterfrom jaswrks:fix/cypress-path-checks
Conversation
|
Why a8c Slack? Was that meant to be a WordPress Slack link? |
|
My bad. I removed it, thanks for pointing that out to me. |
|
It wasn't meant to be a WordPress Slack link, no. The discussion took place in a8c, and I wrongly posted the a8c link without considering the implication. |
| Cypress.Commands.add( 'visitAdmin', ( adminPath ) => { | ||
| cy.visit( '/wp-admin/' + adminPath ).location( 'pathname' ).then( ( path ) => { | ||
| if ( path === '/wp-login.php' ) { | ||
| if ( /\/wp-login\.php$/.test( path ) ) { |
There was a problem hiding this comment.
You could use also endsWith to make it even simpler. Not sure if that matters that much though :)
There was a problem hiding this comment.
Thank you. I will do that.
There was a problem hiding this comment.
@jaswrks @gziolo Actually, I don't think so, at least not in this way. See https://github.com/WordPress/gutenberg/pull/4945/files#r166864232.
|
Thanks, but can we please try to have discussions publicly available, please. If discussions are to take place in Slack then please use the WordPress Slack Workspace that is at least open for anyone to join unlike the Automattic Slack Workspace that is not. p.s https://chat.wordpress.org if you'd like to join the WordPress Slack workspace 😄 |
|
Absolutely, I will be sure to do that in the future. 😃 |
|
Thanks @jaswrks 👍 |
@ntwb This was my fault, I initiated the discussion. I pinged someone privately because I was unsure about the problem I was experiencing. I'll be less shy next time. 😊 |
| Cypress.Commands.add( 'visitAdmin', ( adminPath ) => { | ||
| cy.visit( '/wp-admin/' + adminPath ).location( 'pathname' ).then( ( path ) => { | ||
| if ( path === '/wp-login.php' ) { | ||
| if ( path.endsWith( '/wp-login.php' ) ) { |
There was a problem hiding this comment.
I don't think we polyfill these ATM with Babel. I've been made aware of this several times by @aduth . :) Maybe use the Lodash version here instead?
There was a problem hiding this comment.
Though, this probably runs in the Cypress runner which doesn't have the same constraints as the browser (or all browsers)
There was a problem hiding this comment.
Yes, it's node 8 + electron or Chrome. Travis is happy so are we :)
There was a problem hiding this comment.
Oh, stupid me, I just saw the lines and my alarm bells went off without realising this is Cypress. :D
5 participants
Description
Subtle changes in this PR allow a developer to set a custom base URL for Cypress using an environment variable, where the custom base URL includes a subdirectory. For example, note the
/wpsubdirectory in this command:This wasn't working before. We were running tests against URL paths without considering the possibility of a leading subdirectory in each path. Fixed in this PR.
How Has This Been Tested?
@iseulde and I tested this by running E2E tests in a subdirectory, with help from @youknowriad.
Types of changes
Use regex to test location instead of
===.Checklist: