Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce automated tests #161

Merged
merged 11 commits into from
Nov 3, 2020
Merged

Introduce automated tests #161

merged 11 commits into from
Nov 3, 2020

Conversation

stklcode
Copy link
Contributor

@stklcode stklcode commented Jun 6, 2020

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.

@stklcode stklcode force-pushed the tests branch 3 times, most recently from ecdd5d3 to 345b44d Compare June 10, 2020 13:51
@stklcode stklcode force-pushed the tests branch 2 times, most recently from a1d9c2f to 00d8797 Compare June 16, 2020 18:39
@stklcode stklcode changed the base branch from master to develop July 25, 2020 17:41
@stklcode stklcode force-pushed the tests branch 2 times, most recently from bf6c7fb to ff84a99 Compare October 18, 2020 13:32
@stklcode stklcode marked this pull request as ready for review October 18, 2020 13:33
@florianbrinkmann
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

florianbrinkmann commented Oct 27, 2020

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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

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

@florianbrinkmann
Copy link
Member

florianbrinkmann commented Nov 3, 2020

@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.

@stklcode stklcode merged commit 3d35da2 into develop Nov 3, 2020
@stklcode stklcode deleted the tests branch November 3, 2020 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants