Skip to content

Add Placeholder component tests#935

Merged
BE-Webdesign merged 3 commits intomasterfrom
add/test/components/placeholder
Jun 1, 2017
Merged

Add Placeholder component tests#935
BE-Webdesign merged 3 commits intomasterfrom
add/test/components/placeholder

Conversation

@BE-Webdesign
Copy link
Copy Markdown
Contributor

Adds basic Placeholder component tests. Related to progress on #641.
Adds an extra conditional to make testing easier to prevent an element
that eventually renders to null from sticking in the tree when the icon
prop is not present.

Testing Instructions
Run npm i && npm run test-unit ensure tests pass. Change component logic
to ensure tests fail as they should.

@BE-Webdesign BE-Webdesign added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label May 30, 2017
@BE-Webdesign BE-Webdesign force-pushed the add/test/components/placeholder branch from 701b3f3 to b278f68 Compare May 30, 2017 16:07
@BE-Webdesign
Copy link
Copy Markdown
Contributor Author

BE-Webdesign commented May 30, 2017

Hmm somehow the new build structure broke this. Going to check if the panel icons are broken as well. See #944.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you try instead:

import Placeholder from 'components';

Copy link
Copy Markdown
Contributor Author

@BE-Webdesign BE-Webdesign May 30, 2017

Choose a reason for hiding this comment

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

That is not the problem it is Dashicon. I am switching these tests over to use the proper { Placeholder } from 'components' which does work. There is a dependency on wp.element as an external in Dashicon.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rebased to show that { Placeholder } from 'components' works as expected.

@BE-Webdesign BE-Webdesign force-pushed the add/test/components/placeholder branch from b278f68 to 78ff2bc Compare May 30, 2017 17:26
@nylen
Copy link
Copy Markdown
Member

nylen commented May 30, 2017

Commit b7ce1f1 should fix the build, but really this is a bit of a mess. In test mode an import from 'components' within the components bundle is failing because by the time this import is reached, the bundle isn't defined yet.

I am reasonably satisfied with how the bundling is working in production and development mode, but the test build is a good bit more complicated. We can't use the same multiple-entry-with-multiple-names structure currently, because we require and execute a single test file build/test.js.

I'm inclined to fix this by bringing the test build closer in line with our regular build and executing multiple files, but this can wait until a separate PR.

@BE-Webdesign
Copy link
Copy Markdown
Contributor Author

bundle is failing because by the time this import is reached, the bundle isn't defined yet.

Ah, I thought it had something to do with the use of wp.element as everything else seemed to work fine. Still don't understand how this fixes it but looks good to me for now. Should we merge?

Copy link
Copy Markdown
Member

@nylen nylen left a comment

Choose a reason for hiding this comment

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

:shipit:

BE-Webdesign and others added 3 commits May 31, 2017 20:38
Adds basic Placeholder component tests. Related to progress on #641.
Adds an extra conditional to make testing easier to prevent an element
that eventually renders to null from sticking in the tree when the icon
prop is not present.

Testing Instructions
Run npm i && npm run test-unit ensure tests pass. Change component logic
to ensure tests fail as they should.
@BE-Webdesign BE-Webdesign force-pushed the add/test/components/placeholder branch from b7ce1f1 to ef30f88 Compare June 1, 2017 00:58
@BE-Webdesign BE-Webdesign merged commit 97949eb into master Jun 1, 2017
@BE-Webdesign BE-Webdesign deleted the add/test/components/placeholder branch June 1, 2017 21:54
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.

2 participants