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

Setup Cypress for E2E testing #875

Closed
6 tasks
vikrampm1 opened this issue Apr 20, 2022 · 16 comments · Fixed by #900
Closed
6 tasks

Setup Cypress for E2E testing #875

vikrampm1 opened this issue Apr 20, 2022 · 16 comments · Fixed by #900
Assignees
Labels
help wanted needs:engineering This requires engineering to resolve. type:enhancement New feature or request.
Milestone

Comments

@vikrampm1
Copy link

Is your enhancement related to a problem? Please describe.

See overall concept in the OSBP guide and example setups on Simple Podcasting, Restricted Site Access, and 10up/ElasticPress#2446.

Describe the solution you'd like

  • add base GitHub Action
  • add .wp-env.json
  • add base docker files
  • update package.json / package lock files
  • add cypress tests
  • remove any WP Acceptance components

Designs

n/a

Describe alternatives you've considered

n/a

Additional context

https://github.com/10up/cypress-wp-setup/ && https://github.com/10up/cypress-wp-utils will be helpful in getting this spun up and some initial test commands to leverage.

@vikrampm1 vikrampm1 added type:enhancement New feature or request. needs:engineering This requires engineering to resolve. labels Apr 20, 2022
@vikrampm1 vikrampm1 moved this to Incoming in Open Source Practice Apr 20, 2022
@vikrampm1 vikrampm1 moved this from Incoming to To Do in Open Source Practice Apr 20, 2022
@vikrampm1 vikrampm1 added this to the 1.7.0 milestone Apr 20, 2022
@jeffpaul
Copy link
Member

jeffpaul commented May 6, 2022

Can probably start by taking over #537, yanking out WP Acceptance, and adding in Cypress (along with any existing PHPUnit tests).

@faisal-alvi
Copy link
Member

faisal-alvi commented Jun 23, 2022

@10up/open-source-practice I'm facing the following issues to proceed with Cypress tests (and got stuck), can anyone suggest/help?

  1. Even after adding the following code in the .wp-env.json, a multisite setup is not configured, we get a normal site.
  "core": null,
  "plugins": [".", "./tests/test-plugins"],
  "config": {
      "WP_ALLOW_MULTISITE": true,
   }
  1. As an alternate solution to the above issue, @Sidsector9 recommended using the following script to convert the site into a multisite.
"to-multisite": "wp-env run tests-cli \"wp core multisite-convert --title='Distributor Multisite'\"",

So it worked! (thanks @Sidsector9), but when I added a new site http://localhost:8889/two/ from http://localhost:8889/wp-admin/network/site-new.php, the new site is not working! Probably need to specify all sites in .wp-env.json?

image

  1. The multisite setup was configured successfully as an alternate solution as mentioned above (subsites still not working that issue persists), but it was not working with the latest version (4.9.0) of @wordpress/env, we have used 4.1.1 which works fine. But then Github Action "Dependency Review" failed, mentioning the package [email protected] is vulnerable to "Command Injection via argument injection", that should be upgraded to 3.0.0.

To update it, we need at least 4.5.0 of @wordpress/env! If we upgrade @wordpress/env to 4.5.0 or above, our multisite does not work as mentioned above. Please let me know if anyone can help/suggest here.

image

Seems like if issue 1 is fixed, issues 2 and 3 won't be required anymore.

  1. Distributor requires SSL-certified domains, so I've added a custom plugin (f7a53a9) to disable the SSL (which works in my normal local setup (i.e. 'local by flywheel'), but not in wp-env setup), it throws this error:

image

@Sidsector9
Copy link
Member

Sidsector9 commented Jun 23, 2022

@faisal-alvi for point (2), you can try using wp site create instead of going through the UI way. About the other points, I'll spend some time to investigate.

@faisal-alvi
Copy link
Member

faisal-alvi commented Jun 23, 2022

@Sidsector9 Yes, I will but It's not only about creating a site, I need to literally visit the Distributor plugin setting page, Create posts, Pull, Push, etc.

@dinhtungdu
Copy link
Contributor

dinhtungdu commented Jun 24, 2022

  1. Even after adding the following code in the .wp-env.json, a multisite setup is not configured, we get a normal site.

wp-env hasn't supported installing WP multisite by configuration (see WordPress/gutenberg#27961). So we should use the CLI command suggested by @Sidsector9.=

The main problem here is WordPress multisite can't be installed on domains with custom ports (except 80 and 443) (see image below). There is an ongoing effort to fix this (see the Trac ticket). There are some possible ideas I tested:

  • Modify core. I quickly eliminate this one.
  • Using a MU plugin to support custom port. I found this answer on WordPress SE. But I also gave up on that because it's not a full solution and doesn't work well with the recent WP version.
  • Use port 80 for multisite. This solution may not suitable for running wp-env on local development machines, but it's perfectly fine for testing in CI environments. => This is the only working solution.

image

@dinhtungdu
Copy link
Contributor

Steps to install multisite using wp-env

  • Set the development or tests environment port to 80.
  • Run wp core multisite-convert for the corresponding environment.
  • Update the .htaccess file with rules from here.

@jeffpaul
Copy link
Member

@dinhtungdu do we have that sort of info documented in the utils repo? If not, it would be great to add there for others to hopefully find in the future.

@dinhtungdu
Copy link
Contributor

@dinhtungdu do we have that sort of info documented in the utils repo? If not, it would be great to add there for others to hopefully find in the future.

We don't, I will add a note there.

@faisal-alvi
Copy link
Member

Thanks for the suggestions @dinhtungdu!

Update: As discussed in weekly meeting, we still need to figure out why the wp-env is blocking localhost to verify the SSL, and @dinhtungdu will reproduce the issue and try to figure out the cause when he gets some time.

@dinhtungdu
Copy link
Contributor

dinhtungdu commented Jun 28, 2022

@faisal-alvi I understand your issue now. For the wizard, to avoid the CORS issue, we make requests from the server-side to check the remote site information:

function get_remote_distributor_info() {
if (
! check_ajax_referer( 'dt-verify-ext-conn', 'nonce', false )
|| empty( $_POST['url'] )
) {
wp_send_json_error();
exit;
}
$rest_url = get_rest_url( $_POST['url'] );
if ( is_wp_error( $rest_url ) ) {
wp_send_json_error( $rest_url );
exit;
}
$route = $rest_url . 'wp/v2/dt_meta';
if ( function_exists( 'vip_safe_wp_remote_get' ) && \Distributor\Utils\is_vip_com() ) {
$response = vip_safe_wp_remote_get( $route, false, 3, 3, 10 );
} else {
$response = wp_remote_get( $route, [ 'timeout' => 5 ] );
}
$body = json_decode( wp_remote_retrieve_body( $response ), true );
if ( empty( $body['version'] ) ) {
wp_send_json_error( [ 'rest_url' => $rest_url ] );
exit;
}
wp_send_json_success( array_merge( $body, [ 'rest_url' => $rest_url ] ) );
exit;
}

As @peterwilsoncc pointed out in the meeting, making a request from the webserver to localhost:8888 won't work because it always points to the WP container itself.

IMO, there are two possible solutions:

  1. Use custom domain mapping. Instead of using localhost, we use a custom domain for the environment. We will also need to add the domain mapping entry to the hosts file on the GitHub Actions as well as the local development machine.
  2. Use the old trick we used for E2E testing with WP Acceptance. All we need is a subdir multisite. Then we add external connections using the sub-sites.

The first one requires a more complex setup but will replicate closer to the real installation. The second solution sacrifices the real use case but is a much simpler setup.

What do you think? Which solution do you prefer?

@faisal-alvi
Copy link
Member

@dinhtungdu I prefer the second solution. However, we need a subdir multisite inside wp-env, that is the main issue!

@dinhtungdu
Copy link
Contributor

However, we need a subdir multisite inside wp-env, that is the main issue!

@faisal-alvi As I mentioned above, I set up successfully a subdirectory multisite with wp-env by using port 80. Can you try these steps when you have time?

@faisal-alvi
Copy link
Member

@dinhtungdu yes got it, I will try as soon as I get some time to work on this ticket. Thank you, Tung!

@faisal-alvi
Copy link
Member

@dinhtungdu - I've tried the steps mentioned above.

  • Stopped "local by flywheel".
  • Updated wp-env.json:
{
  "core": null,
  "plugins": [".", "./tests/test-plugins"],
  "config": {
    "WP_ALLOW_MULTISITE": true,
    "WP_ENV_TESTS_PORT": 80
  },
  "env": {
    "tests": {
      "mappings": {
        "wp-cli.yml": "./tests/bin/wp-cli.yml"
      },
      "WP_ENV_TESTS_PORT": 80
    }
  }
}
  • Started wp-env environment.
  • Converted to multisite using WP-CLI.
  • Updated .htacess using: npm run env run cli vi .htaccess

I'm still seeing http://localhost8889/ working instead of the 80 port. Also, the subsites in it does not work.

@dinhtungdu
Copy link
Contributor

I'm still seeing http://localhost8889 working instead of the 80 port. Also, the subsites in it does not work.

@faisal-alvi You can set the port as follows. Subsites won't work if your port isn't 80.

{
  "env": {
    "tests": {
      "port": 80
    }
  }
}

@peterwilsoncc
Copy link
Collaborator

I'll move this to the 2.0.0 milestone as 1.7.0 likely to be released early next week.

@peterwilsoncc peterwilsoncc modified the milestones: 1.7.0, 2.0.0 Jul 21, 2022
Repository owner moved this from In Progress to Merged in Open Source Practice Aug 2, 2022
@peterwilsoncc peterwilsoncc modified the milestones: 2.0.0, 1.7.1 Aug 4, 2022
@vikrampm1 vikrampm1 moved this from Merged to Done/Released in Open Source Practice Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted needs:engineering This requires engineering to resolve. type:enhancement New feature or request.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants