Skip to content
This repository has been archived by the owner on Feb 18, 2021. It is now read-only.

Using a single file per Gutenberg post/page tests #1682

Closed
wants to merge 18 commits into from

Conversation

JavonDavis
Copy link
Contributor

Following up on a something in a previous conversation....

Right now we have 3 separate files for Gutenberg e2e tests wp-post-editor-spec, wp-gutenberg-post-editor-spec and wp-calypso-gutenberg-post-editor-spec that all test the same thing across different environments: Classic Editor, Gutenberg in WPCOM and dev environment /gutenberg, do you think we should see how we could use some kind of config and a single test file? I think it can work at least for the different Gutenberg setups... I think the Goal is to eventually only have tests against the actual environment Gutenberg in Calypso right? this kind of setup might make it easier when transitioning to that and maybe switch between testing on the different environments with minimal work in the future.

I explored a few different ways to reduce what we have but this is what I settled on so far and could use some feedback...

Two main changes

  • Removed wp-calypso spec files and used environment variable WPCALYPSO to decide the environment and the steps to take at login
  • Didn't update run script to test against this environment in the CI, I'm thinking the approach of having the ability to write code for this locally and basically switch it off until it's ready on dotcom is sufficient coverage, having tests that specifically target in the suite wpcalypso seemed a bit weird. Thoughts on this?

To Test:

Run wp-gutenberg-post-editor-spec.js and wp-gutenberg-page-editor-spec.js normally and it should only run against wordpress.com

Run wp-gutenberg-post-editor-spec.js and wp-gutenberg-page-editor-spec.js normally with WPCALYPSO set to true and it should only run against wpcalypso.

@JavonDavis JavonDavis self-assigned this Nov 30, 2018
@JavonDavis JavonDavis requested a review from a team November 30, 2018 20:52
@alisterscott
Copy link
Contributor

Thanks for looking at this @JavonDavis

Didn't update run script to test against this environment in the CI, I'm thinking the approach of having the ability to write code for this locally and basically switch it off until it's ready on dotcom is sufficient coverage, having tests that specifically target in the suite wpcalypso seemed a bit weird. Thoughts on this?

I think we need to be able to run these against wpcalypso as we transition to using Gutenberg in Calypso it will give us confidence that everything is still working - as Gutenberg in Calypso works technically a lot different to wp-admin

Is there a way for CI to still run both?

@Stojdza
Copy link
Contributor

Stojdza commented Dec 3, 2018

Well done @JavonDavis! I like this simplification and maintain only one spec instead of two

@JavonDavis
Copy link
Contributor Author

Is there a way for CI to still run both?

Yeah, I've updated the script to do that but there are some failures now that I'm looking into the cause. The options I had in mind was this update I've made so far to the run script to run the wpcalypso tests or an update to the bridge repo to run the wpcalypso tests separately. I decided to give it a go with the script first and see if that can work well.

@alisterscott
Copy link
Contributor

I think we can hold off on this one since we'll be going all in on Gutenberg in Calypso in about a day or so, and this won't be necessary as WordPress.com will simply use Gutenberg in Calypso for anyone opted in, and we can just delete the wp-calypso spec

@alisterscott
Copy link
Contributor

This is the PR to enable Gutenberg for Calypso: Automattic/wp-calypso#29121

@Stojdza
Copy link
Contributor

Stojdza commented Dec 5, 2018

and we can just delete the wp-calypso spec

We should be careful here just to don't miss something since specs are not completely identical. For example, in wp-calypso 'Trash Post: and Insert a contact form: are disabled, and in wp-gutenberg Trash Post: and Insert a payment button: are disabled. Also, we have Insert embeds: in wp-calypso.

@JavonDavis
Copy link
Contributor Author

@Stojdza yeah when the time comes we should probably copy those over and disable them until it's ready for Gutenberg in Calypso just to save some time. The version of the wp-gutenberg in this PR should actually have all of those but checked based on a condition I could rework this PR remove all the environment stuff, or should we work with a new PR for that?

@Stojdza
Copy link
Contributor

Stojdza commented Dec 5, 2018

@JavonDavis I think that new PR from the fresh new master branch would be safer, but basically, there is no much difference.

@Stojdza
Copy link
Contributor

Stojdza commented Dec 6, 2018

@JavonDavis I think that we are ok now with these 3 PRs: #1697 #1694 #1695
We only need to be aware of disabled Trash post test.

@Stojdza Stojdza deleted the try/gutenberg-environment-config branch December 7, 2018 15:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants