Skip to content

Ignore Story editor tests#4043

Merged
westonruter merged 4 commits intodevelopfrom
remove-e2e-test
Jan 10, 2020
Merged

Ignore Story editor tests#4043
westonruter merged 4 commits intodevelopfrom
remove-e2e-test

Conversation

@pierlon
Copy link
Copy Markdown
Contributor

@pierlon pierlon commented Jan 8, 2020 •
edited
Loading

Summary

This PR ignores the Stories Editor E2E tests due to their flakiness.

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@googlebot googlebot added the cla: yes Signed the Google CLA label Jan 8, 2020
@pierlon pierlon requested a review from westonruter January 8, 2020 23:04
westonruter
westonruter requested changes Jan 8, 2020
View reviewed changes
.travis.yml
# PHP unit tests (7.3, WordPress trunk)
- env: WP_VERSION=trunk DEV_LIB_ONLY=phpunit INSTALL_PWA_PLUGIN=1
# PHP and JavaScript unit tests (7.3, WordPress trunk, with code coverage)
- env: WP_VERSION=latest DEV_LIB_ONLY=phpunit INSTALL_PWA_PLUGIN=1 RUN_PHPUNIT_COVERAGE=1
Copy link
Copy Markdown
Member

@westonruter westonruter Jan 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swissspidy Should this still be allowed to fail? Why was added initially?

Copy link
Copy Markdown
Collaborator

@swissspidy swissspidy Jan 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because trunk is not guaranteed to be stable. #4029 just showed it this week :-)

See #2408 for our discussion about adding this test & marking it as allowed to fail.

@westonruter westonruter added this to the v1.5 milestone Jan 9, 2020
@pierlon
Copy link
Copy Markdown
Contributor Author

pierlon commented Jan 9, 2020

PHP 7.4.0 on Travis does not include the gd extension, causing errors to be thrown. I've reported it on their forum.

@pierlon
Copy link
Copy Markdown
Contributor Author

pierlon commented Jan 9, 2020

⚠️ Rebasing to remove reverted commits.

@pierlon pierlon changed the title Remove Story E2E test job Ignore Story & Block editor tests Jan 9, 2020
@pierlon pierlon requested a review from westonruter January 9, 2020 22:56
westonruter
westonruter reviewed Jan 10, 2020
View reviewed changes
@pierlon pierlon changed the title Ignore Story & Block editor tests Ignore Story editor tests Jan 10, 2020
@westonruter
Copy link
Copy Markdown
Member

westonruter commented Jan 10, 2020

Should this not also remove:

amp-wp/.travis.yml

Line 71 in 1f842d3

- env: WP_VERSION=latest DEV_LIB_SKIP=phpcs,eslint,xmllint,phpsyntax,phpunit PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=

@pierlon
Copy link
Copy Markdown
Contributor Author

pierlon commented Jan 10, 2020

Yes, nice catch.

@westonruter westonruter modified the milestones: v1.5, v1.4.3 Jan 10, 2020
@westonruter westonruter merged commit a7e0497 into develop Jan 10, 2020
@westonruter westonruter deleted the remove-e2e-test branch January 10, 2020 16:59
westonruter pushed a commit that referenced this pull request Jan 10, 2020
@pierlon @westonruter
Ignore Story editor tests (#4043)
dd8cb38 
* Ignore block-editor & stories-editor E2E tests

* Conform to semver updated method signatures

* Fix failing block-editor tests

* Disallow E2E tests to fail
@westonruter
Copy link
Copy Markdown
Member

westonruter commented Jan 10, 2020

I cherry-picked the changes here onto the 1.4 branch, but the E2E are failing for some reason: https://travis-ci.org/ampproject/amp-wp/jobs/635359942

Any ideas?

westonruter added a commit to xwp/amp-wp that referenced this pull request Jan 10, 2020
@westonruter
Merge branch 'develop' of github.com:ampproject/amp-wp into add/spa-a…
8caa8db 
…mp-shadow-dom-updated

* 'develop' of github.com:ampproject/amp-wp: (233 commits)
  Ignore Story editor tests (ampproject#4043)
  Update dependency eslint-plugin-jest to v23.4.0 (ampproject#4056)
  Update dependency xwp/wp-dev-lib to v1.4.0
  Update amp-video embed regex pattern to include other Vimeo URL formats (ampproject#4051)
  Update amp-instagram embed regex (ampproject#4053)
  Update dependency terser-webpack-plugin to v2.3.2 (ampproject#4047)
  Fix ability to update the status of a validation error
  Update wp-dev-lib package (ampproject#4029)
  Update dependency core-js to v3.6.2 (ampproject#4028)
  Update dependency @wordpress/element to v2.10.0 (ampproject#3986)
  Update dependency browserslist to v4.8.3 (ampproject#4021)
  Update dependency grunt-http to v2.3.3 (ampproject#4022)
  Update dependency postcss to v7.0.26 (ampproject#4017)
  Update dependency eslint-utils to v2 (ampproject#4012)
  Update dependency eslint-plugin-jest to v23.3.0 (ampproject#4016)
  Update dependency css-loader to v3.4.1 (ampproject#4023)
  Update dependency webpack to v4.41.5 (ampproject#4014)
  Update dependency core-js to v3.6.1 (ampproject#4011)
  Update dependency eslint to v6.8.0 (ampproject#4002)
  Swap PHP 5.4 for 5.6 to run external-http tests
  ...
@pierlon
Copy link
Copy Markdown
Contributor Author

pierlon commented Jan 11, 2020

The Gutenberg plugin failed to activate, which led to tests failing due to them being unable to find the case-sensitive words "Featured image" (see 1f842d3#diff-792685570ec094fec9f40f84e3af8d3bR25). Apparently WP CLI failed to install and activate Gutenberg due to clash in a function declaration (WP_CLI\UpgraderSkin::feedback($string) vs WP_Upgrader_Skin::feedback($string, ...$args).

On the 1.4 branch, WP CLI v2.3.0 is installed via Composer while within the Docker container v2.4.0 is used. Further investigation shows v2.3.0 uses WP_CLI\UpgraderSkin::feedback($string) while v2.4.0 on PHP > 5.6 (which is the case here) uses WP_Upgrader_Skin::feedback($string, ...$args), which leads me to speculate whether both versions are being loaded at the same time. Thoughts @schlessera?

The develop branch uses WP CLI v2.4.0 in both instances, however, which I think is why the error does not occur there.

@schlessera
Copy link
Copy Markdown
Collaborator

schlessera commented Jan 17, 2020

This is due to a breaking change in WordPress Core 5.3. While making the move to PHP 5.6 code, the signature for the upgrader base class feedback() method changed from WP_Upgrader_Skin::feedback($string) to WP_Upgrader_Skin::feedback($string, ...$args).

WP-CLI 2.3.0 is compatible with older versions of WP (< 5.3) only, while WordPress Core 5.3+ requires WP-CLI 2.4.0+.

This is unfortunate, but that's WordPress' way of backward compatibility I'm afraid...

@schlessera
Copy link
Copy Markdown
Collaborator

schlessera commented Jan 17, 2020

In case that wasn't clear, the solution is to use WP-CLI v2.4.0+ in the Docker container as well, as it seems to be using WP Core 5.3+.

@schlessera
Copy link
Copy Markdown
Collaborator

schlessera commented Jan 17, 2020

No, the other way around, install WP-CLI 2.4.0+ on the v1.4 branch if you're using WP Core 5.3+ there. 🤪

@westonruter westonruter mentioned this pull request Jan 25, 2020
Closed
@westonruter
Copy link
Copy Markdown
Member

westonruter commented Jan 25, 2020

Moving this to a separate issue: #4174.

westonruter added a commit that referenced this pull request Feb 13, 2020
@westonruter
Merge tag '1.4.3'
06162df 
* tag '1.4.3': (22 commits)
  Update readme and screenshots for Stories removal (#4259)
  Open story export instructions in a new window (#4258)
  Bump version to 1.4.3-RC1
  Hide Stories options and add deprecation notice (#4219)
  Fix malformed conversion of relative action URLs for forms (#4250)
  Limit Stories experience to WP 5.3 & Gutenberg 7.1.0 (#4217)
  Prevent errors in admin bar filters from non-array arguments (#4207)
  Update @wordpress/e2e-test-utils dependency
  Revert update of mustache/mustache dependency
  Update composer.lock
  Update WP CLI to 2.4.0
  For WordPress.tv embed, Use an oembed filter instead of block filter (#4164)
  Update readme to add FAQs section (#4159)
  Apply workaround to fix test__multiple_valid_image_files (#4034)
  Ignore Story editor tests (#4043)
  Update amp-video embed regex pattern to include other Vimeo URL formats (#4051)
  Update amp-instagram embed regex (#4053)
  Update wp-dev-lib package (#4029)
  Fix conversion of forms with relative action URLs (#4003)
  Improve release instructions (#3995)
  ...
delawski added a commit to xwp/amp-wp that referenced this pull request Jul 22, 2020
@delawski
Merge branch 'add/spa-amp-shadow-dom' into xwp/add/spa-amp-shadow-dom
22cff74 
* add/spa-amp-shadow-dom: (3410 commits)
  Improve copy in comment and error message
  Reuse constant
  Revert introduction of amp_document_output filter for now
  attachShadowDocAsStream expects a string type variable
  AppShell polyfill: Update to latest version - 2.4.1
  Add a filter to allow altering AMP output from themes
  Ensure .site-content-contain element exists
  Amp-Wp-App-Shell - Improve flexibility for different types of caching
  AppShell polyfill: Update to latest version - 2.4.0
  Fix eslint issues
  Fix build and unit tests by preserving uncompiled scripts
  Make use of new Amp\AmpWP\Document methods; add line breaks
  Restore missing live_list_offline_commenting to AMP_Service_Worker
  Fix phpcs linting issues
  Add missing changes to style sanitizer; remove duplicated autoloader entry
  Ignore Story editor tests (ampproject#4043)
  Update dependency eslint-plugin-jest to v23.4.0 (ampproject#4056)
  Update dependency xwp/wp-dev-lib to v1.4.0
  Update amp-video embed regex pattern to include other Vimeo URL formats (ampproject#4051)
  Update amp-instagram embed regex (ampproject#4053)
  ...

# Conflicts:
#	composer.json
delawski added a commit to xwp/amp-wp that referenced this pull request Jul 22, 2020
@delawski
Merge branch 'xwp/add/spa-amp-shadow-dom' into xwp/nova-app-shell
04eb817 
* xwp/add/spa-amp-shadow-dom: (250 commits)
  Improve copy in comment and error message
  Reuse constant
  Revert introduction of amp_document_output filter for now
  attachShadowDocAsStream expects a string type variable
  AppShell polyfill: Update to latest version - 2.4.1
  Add a filter to allow altering AMP output from themes
  Ensure .site-content-contain element exists
  Amp-Wp-App-Shell - Improve flexibility for different types of caching
  AppShell polyfill: Update to latest version - 2.4.0
  Fix eslint issues
  Fix build and unit tests by preserving uncompiled scripts
  Make use of new Amp\AmpWP\Document methods; add line breaks
  Restore missing live_list_offline_commenting to AMP_Service_Worker
  Fix phpcs linting issues
  Add missing changes to style sanitizer; remove duplicated autoloader entry
  Ignore Story editor tests (ampproject#4043)
  Update dependency eslint-plugin-jest to v23.4.0 (ampproject#4056)
  Update dependency xwp/wp-dev-lib to v1.4.0
  Update amp-video embed regex pattern to include other Vimeo URL formats (ampproject#4051)
  Update amp-instagram embed regex (ampproject#4053)
  ...

# Conflicts:
#	Gruntfile.js
#	assets/js/amp-wp-app-shell.js
#	includes/amp-helper-functions.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonruter westonruter westonruter approved these changes

@swissspidy swissspidy swissspidy approved these changes

Assignees

No one assigned

Labels

cla: yes Signed the Google CLA

Projects

None yet

Milestone

v1.4.3

Development

Successfully merging this pull request may close these issues.

5 participants

@pierlon @westonruter @schlessera @swissspidy @googlebot