Skip to content

Conversation

@gziolo
Copy link
Member

@gziolo gziolo commented Nov 30, 2018

Description

Part of #12313.

This PR moves all e2e test files to the new npm package called @wordpress/tests-e2e. This is the first step which should allow us to use those tests to verify WordPress core. Next step will require to extract expose all tests plugins and utils to their own package and make sure that they can be run from the node_modules folder. Jest by default ignore all files located in the folder which contains installed npm packages.

How has this been tested?

npm run test-e2e

@gziolo gziolo added [Status] In Progress Tracking issues with work in progress [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. labels Nov 30, 2018
@gziolo gziolo self-assigned this Nov 30, 2018
@gziolo gziolo force-pushed the add/tests-e2e-package branch 4 times, most recently from 67542d7 to 9046959 Compare December 7, 2018 12:27
@gziolo gziolo force-pushed the add/tests-e2e-package branch from 9046959 to 9f3068b Compare December 9, 2018 16:06
@gziolo gziolo removed the [Status] In Progress Tracking issues with work in progress label Dec 9, 2018
@gziolo gziolo added this to the 4.8 milestone Dec 9, 2018
@gziolo gziolo requested a review from a team December 9, 2018 20:28
"rootDir": "../../",
"preset": "jest-puppeteer",
"setupTestFrameworkScriptFile": "<rootDir>/test/e2e/support/setup-test-framework.js",
"setupTestFrameworkScriptFile": "<rootDir>/packages/tests-e2e/support/setup-test-framework.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

this path seems like it'd be problematic when test-e2e is installed as a node module. Is this something you're planning on tackling in a later PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good point. The same issue applies to the root directory. I think it's easier to revisit once both #12437 and #12465 are merged. I feel like having a follow-up PR with changes which try to make it reusable outside is going to be easier. For the time being, I feel like we only need to make this package private to ensure it isn't exposed to npm as it clearly isn't fully ready. However, I want to move forward so don't have to deal with the folder structure and rebases forever.

Copy link
Member Author

Choose a reason for hiding this comment

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

This file was removed so it's sort of a non-issue at the moment. However, I want to explore whether this is something we want to include in the default e2e config. We would need to expose e2e utils first, to be able to reference those two though.

@oandregal
Copy link
Member

If the goal is to let others (core, plugin/theme authors, etc) reuse and execute these tests, they will also need a setup. Gutenberg setup is being extracted at #12437 as a different package, and I wonder: what's the benefit of having two different packages if the two are required (and dependant)? Do we need to run these e2e tests with a different setup? Would a single package for tests and setup be enough?

@gziolo
Copy link
Member Author

gziolo commented Dec 10, 2018

what's the benefit of having two different packages if the two are required (and dependant)? Do we need to run these e2e tests with a different setup? Would a single package for tests and setup be enough?

wp-scripts test-e2e is a general purpose script which is meant to run any Jest + Puppeteer tests. It's targeting plugin developers who want to add tests for their code. That's why we have to offer our test utils so they don't need to copy them. You could argue whether they should be in their own package or can be bundled with core tests. I assumed it's nice to have utils as a separate package. I don't feel strongly about that though.

I also anticipate that there might be 3 different config related setups required to make it all work as intended. We might need 2 or 3 files:

  1. Gutenberg configuration to run all e2e tests.
  2. Config to run all e2e tests directly from the npm package.
  3. A default config used by plugins if they don't provide anything.
    Hopefully, we can consolidate 1 & 2 into one thing.

@gziolo gziolo force-pushed the add/tests-e2e-package branch from ddaed42 to aeaa234 Compare December 12, 2018 13:25
@talldan
Copy link
Contributor

talldan commented Dec 14, 2018

@gziolo Instead of running tests directly from node_modules and needing extra configuration to make that happen, I wondered whether an option could be to instead import the tests from the package into a file that's not in node modules. e.g in our tests/e2e/index.js file we'd just have import '@wordpress/tests-e2e';.

The index.js in the tests-e2e package would need to import each of the test files for it to work.

@@ -0,0 +1,37 @@
{
"name": "@wordpress/tests-e2e",
Copy link
Contributor

Choose a reason for hiding this comment

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

One other thought on this, I was wondering if tests-e2e might be too much of a generic name. I'm not sure if we'd consider having more than just gutenberg/block-editor tests in this package.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree that we should have more granular control on what tests can be run. I expect this to grow past the editor only. At some point we'd be able to run phase2 tests, core editor tests, ... separately. It could be just . import { editorTests } from "@wordpress/tests-e2e".

I don't have a strong opinion or a perfect solution at the moment though, this is probably something we'd have to iterate on.

Copy link
Member Author

Choose a reason for hiding this comment

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

Jest support projects, so we can split it into smaller groups using this feature:

https://jestjs.io/docs/en/configuration#projects-array-string-projectconfig

@oandregal
Copy link
Member

@gziolo I'm still on the fence regarding how to distribute utils and core tests (bundled with the wp-scripts package or separate utilities). I'm admittedly less exposed to other use cases (WordPress core, 3rd party WordPress setups, plugins, etc), so I'll let people with that kind of experience chime in and offer advice.

@youknowriad youknowriad modified the milestones: 4.8, 4.9 Dec 19, 2018
@gziolo gziolo force-pushed the add/tests-e2e-package branch from aeaa234 to 5391a68 Compare January 7, 2019 10:19
@gziolo gziolo requested a review from adamsilverstein January 7, 2019 10:27
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

It's looking good for me as a start. We should test it in Core and see from there.
The only concern I have is about the name
Is this package going to include all WordPress e2e tests? If it's the case, the name is good
otherwise, we might need to be specific

@gziolo
Copy link
Member Author

gziolo commented Jan 7, 2019

The only concern I have is about the name
Is this package going to include all WordPress e2e tests? If it's the case, the name is good
otherwise, we might need to be specific

I would prefer to keep all test suites in one package, but find a way to make it easy to run the subset of them using a specific folder structure or projects option built-in in Jest.

@gziolo gziolo merged commit afb6afe into master Jan 7, 2019
@gziolo gziolo deleted the add/tests-e2e-package branch January 7, 2019 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants