Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

Add the first integration test #62

Closed
wants to merge 1 commit into from
Closed

Add the first integration test #62

wants to merge 1 commit into from

Conversation

mvasin
Copy link

@mvasin mvasin commented Mar 10, 2018

This PR adds the first test that actually interacts with WordPress core.

I think that stubs make sense in cases like interacting with external APIs. When tests involve WordPress core, there is no instability, and such broader tests (potentially with database involved) could run a bit longer, but they also test much more, so setting them up is worth the hassle.

I tried to touch the current code as little as possible, but I think that eventually we should rewrite tests to get rid of stubs. That's why I moved phpunit out of composer.json to tests/composer.json.

After/if we agree on this, I will add tests and implement the optional enforcing of SSL response when WordPress is addressed from non-443 port (WP_FORCE_SSL environment variable, see vinkla/wordplate#196).

What I'm currently stuck with is coverage reports. The reporter does generate analysis, and I map tests/vendor/wordplate/framework/src directory to src/ according to the docs, but still codecov sees nothing.

Add the first test that actually interacts with WordPress core.
@vinkla
Copy link
Contributor

vinkla commented Mar 12, 2018

Thanks for the pull request @mvasin! I'll take a look as soon as possible.

@@ -17,6 +17,8 @@
use WordPlate\Application;
use WordPlate\Container;

require_once __DIR__.'/stubs.php';
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, maybe we can autoload this file instead of using require?

@@ -54,6 +43,9 @@
"dev-master": "6.2-dev"
}
},
"scripts": {
"test": "@composer --working-dir=tests test"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this with PHPUnit in the .travis.yml file?

</whitelist>
</filter>
<php>
<env name="WP_ENV" value="testing"/>
</php>
<logging>
<log type="coverage-clover" target="clover.xml"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to move this here? I would prefer if this was added to the .travis.yml file instead.

@vinkla
Copy link
Contributor

vinkla commented Mar 12, 2018

I tried to touch the current code as little as possible, but I think that eventually we should rewrite tests to get rid of stubs.

I agree, it would be great if we could use the built-in functions.

That's why I moved phpunit out of composer.json to tests/composer.json.

Do we really need to keep these separate? It would be great if we could keep one composer.json file.

...but still codecov sees nothing.

I wonder if Codecov can handle WordPress 🤔

@mvasin
Copy link
Author

mvasin commented Apr 7, 2018

@vinkla For some reason I missed notifications about your response, and currently I'm busy, but I'll get back to this as soon as I can.

@vinkla
Copy link
Contributor

vinkla commented Apr 7, 2018

@mvasin no worries, there is no stress 😉

@vinkla
Copy link
Contributor

vinkla commented Nov 28, 2018

Thanks for the pull request @mvasin! I'm closing this due to inactivity.

@vinkla vinkla closed this Nov 28, 2018
@mvasin
Copy link
Author

mvasin commented Nov 28, 2018

I've been keeping this PR in my head and it's pity I couldn't find time to finish it. If someone (including me) will take this effort further later on, have a look at WordPress/gutenberg#12313, it could help.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

2 participants