Skip to content

Conversation

@kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Aug 11, 2020

Replaces and closes #16809

Changes proposed in this Pull Request:

  • Adds a script to run package unit tests within the Docker env.
  • Renames all packages that had phpunit.xml to phpunit.xml.dist.

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

n/a

Testing instructions:

  • yarn docker:up
  • yarn docker:phpunit:package autoloader to test a specific package.
  • yarn docker:phpunit:package to test all of them.

Proposed changelog entry for your changes:

@kraftbj kraftbj added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. Unit Tests Docker [Focus] Jetpack DNA labels Aug 11, 2020
@kraftbj kraftbj requested a review from dereksmart August 11, 2020 19:53
@kraftbj kraftbj self-assigned this Aug 11, 2020
Copy link

@test-case-reminder test-case-reminder bot left a comment

Choose a reason for hiding this comment

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

Here are some suggested test cases for this PR.

Connection

  • In-place connection with free plan
  • In-place connection with paid plan
  • In-place connection with product purchase
  • Classic connection. Use Safari, or set a constant JETPACK_SHOULD_NOT_USE_CONNECTION_IFRAME to true
  • Disconnect/reconnect connection
  • Secondary user connection
  • Connection on multisite

If you think that suggestions should be improved please edit the configuration file here. You can also modify/add test-suites to be used in the configuration file.

@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D47977-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@github-actions github-actions bot added the [Status] Needs Package Release This PR made changes to a package. Let's update that package now. label Aug 11, 2020
@kraftbj
Copy link
Contributor Author

kraftbj commented Aug 11, 2020

Running all of them takes a bit, so may want to add a "you didn't pass a package name, do you want to run them all?" check. Hmm.

@jetpackbot
Copy link
Collaborator

jetpackbot commented Aug 11, 2020

Warnings
⚠️

pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16810

Generated by 🚫 dangerJS against 9882bcc

@jeherve jeherve added this to the 8.9 milestone Aug 12, 2020
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Aug 12, 2020
@kraftbj kraftbj merged commit 6d2a95c into master Aug 12, 2020
@kraftbj kraftbj deleted the add/package-test-script branch August 12, 2020 12:56
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Aug 12, 2020
davidlonjon added a commit that referenced this pull request Aug 14, 2020
* master: (41 commits)
  use blog token to make the request (#16635)
  External Media: Add account disconnect button (#16759)
  CI: Try collect js coverage (#16786)
  Sync: Fix nonce action string in theme edit sync (#16702)
  Connect-in-place: hide new heading during connection process (#16703)
  Update dependency eslint-plugin-jsdoc to v30.2.1 (#16765)
  Theme Tools: Resolve PHP 7.4 array offset notice. (#16795)
  New shell command for easier access to the database. (#16761)
  My Plan: Add Offer Reset project new plans (Jetpack Security, Jetpack Complete) (#16739)
  Increase the `editor.MediaUpload` hook priority (#16669)
  External Media: Remove `speak` announcement when inserting media.
  Extensions: make `render_callback` optional when checking block registration against plan (#16746)
  Conditional check for wrapper before giving focus to new page (#16817)
  Docker: Add package testing shortcut (#16810)
  Settings: Recognize valid Akismet keys from wp-config and restrict input (#16542)
  Social Previews: Add Modal (#16704)
  Update dependency preact to v10.4.7 (#16768)
  Improve a11y of amp-social-share (#16737)
  Instant Search: Tweak expanded result path styling (#16762)
  Docker: Add phpmyadmin to the docker-composer.yml (#16806)
  ...
@jeherve
Copy link
Member

jeherve commented Aug 14, 2020

@kraftbj Following-up on this; since we've renamed the phpunit files, I think we'll need to update the .gitattributes files for each package accordingly.

@kraftbj
Copy link
Contributor Author

kraftbj commented Aug 14, 2020

Oh good catch. I'll PR that today.

kraftbj added a commit that referenced this pull request Aug 14, 2020
kraftbj added a commit that referenced this pull request Aug 14, 2020
@jeherve
Copy link
Member

jeherve commented Aug 24, 2020

r212536-wpcom

pereirinha pushed a commit that referenced this pull request Sep 10, 2020
* Add ability to run package tests via Docker

* [not verified] Packages: update readme to include phpunit docs

* Add package testing information

* Add prompt if no package name is passed.

Co-authored-by: Derek Smart <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docker [Focus] Jetpack DNA [Status] Needs Package Release This PR made changes to a package. Let's update that package now. Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it Unit Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants