pluginkollektiv / statify

Statify – statistics plugin for WordPress
https://wordpress.org/plugins/statify/
GNU General Public License v3.0
77 stars 23 forks source link

Introduce automated tests #161

Closed stklcode closed 4 years ago

stklcode commented 4 years ago

This PR introduces unit and integration tests for Statify.

Tests run against test setups for PHP 5.6 / WP 4.7 and PHP 7.4 / WP 5.5, all other builds only lint the code.

florianbrinkmann commented 4 years ago

I tested the commands from .travis.yml locally with WSL2 and ddev and composer test works fine, so I guess the PR is ready to be merged, or is there something other that needs to be checked?

stklcode commented 4 years ago

Well, the tests themselves should do something relevant of course. self::assertTrue(true) will also run without issues but obviously wouldn't tell us anything.

I'd say my test cases are somewhat reasonable, hopefully documented well enough to get an idea of what is tested and the code-style can be verified with PHPCS (some rule adaptions, as WPCS is not perfectly applicable for test cases). If you agree, we're set here. If not, feel free to suggest changes.

florianbrinkmann commented 4 years ago

Ah, sorry, I thought the PR was about that the tests run as part of the CI now. Will take a look at the tests, I try to do that later today.

stklcode commented 4 years ago

Great đź‘Ť Up to the current version we follow a "manual testing approach", i.e. there are no programmatic tests at all. Tried to put my typical manual tests into an automated integration test suite.

florianbrinkmann commented 4 years ago

I looked at the tests and think I got a good idea of what they are doing. I have no suggestion for changes/additional tests. Two tests are failing, should that be fixed in the PR or in separate issues?


1) Test_Cron::test_cronjob
Statify cleanup cron job should not be registered in normal cycle
Failed asserting that 10 is false.

/var/www/html/statify/tests/test-cron.php:36

2) Test_Dashboard::test_get_stats
Unexpected 1st referrer URL
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'https://statify.pluginkollektiv.org/'
+'https://statify.pluginkollektiv.org/documentation/'

/var/www/html/statify/tests/test-dashboard.php:182

The files in the tests directory are not checked by PHPCS, but I guess that is what you mean with »some rule adaptions, as WPCS is not perfectly applicable for test cases«?

stklcode commented 4 years ago

Two tests are failing, should that be fixed in the PR or in separate issues?

The tests assumed that the cron task is only scheduled during cron calls, but that recently changed in #187. I adjusted the corresponding test case.

The files in the tests directory are not checked by PHPCS, but I guess that is what you mean with »some rule adaptions, as WPCS is not perfectly applicable for test cases«?

Thought code style checks on test code might be little pedantic... Can be verified with composer lint-tests with a separate PHPCS configuration (passing without warnings).

florianbrinkmann commented 4 years ago

The tests assumed that the cron task is only scheduled during cron calls, but that recently changed in #187. I adjusted the corresponding test case.

Okay, cool. The second test continues to fail for me:

Unexpected 1st referrer URL
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'https://statify.pluginkollektiv.org/'
+'https://statify.pluginkollektiv.org/documentation/'

/var/www/html/statify/tests/test-dashboard.php:182

Thought code style checks on test code might be little pedantic... Can be verified with composer lint-tests with a separate PHPCS configuration (passing without warnings).

Okay :) Just wanted to make sure it was not unintentionally.

stklcode commented 4 years ago

Interesting, runs fine on my local (Linux) machine and Travis CI :thinking:

Guess it might be a different execution order. I'll try to figure it out and probably add some additional DB cleanups.

florianbrinkmann commented 4 years ago

@stklcode after running the test again I got the same error, but now in line 239. So I added your modifications there, too, and now all tests pass.