-
Notifications
You must be signed in to change notification settings - Fork 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
Remove e2e tests for post visibility #90207
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
Pinging @ntsekouras because you put out the origin PR. Would it be appropriate to remove this test? Are there plans to migrate them instead since as mentioned here, the UI is being moved elsewhere. |
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
Now we are using the new Post Status dialog to explicitly handle the post status. So you could either remove these tests or update them to test whatever, but using the the new component. For example in order to set a password, you can do it through the Post Status component. |
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.
Looks good to me. I'm going to approve that pull request - I'll let you decide if it's better to remove those tests rather than updating them as Nik mentioned it.
There old UI is so far off from the new UI that the effort it takes to have tests that work across both versions is considerable. I'm going to go ahead with my initial decision to remove the tests and them introduce them after the upgrade. |
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
Fixes failing e2e tests that arose as a result of the post visibility component that was removed in this PR ( commit ).