From 13b4d3d3055b08b710197452f81414c9bf09a462 Mon Sep 17 00:00:00 2001 From: Eric Lippmann Date: Wed, 19 Nov 2014 16:11:49 +0100 Subject: [PATCH] Remove doc/test/* --- doc/test/php_tests.md | 65 ------- doc/test/running_tests.md | 88 ---------- doc/test/styleguide.md | 359 -------------------------------------- 3 files changed, 512 deletions(-) delete mode 100644 doc/test/php_tests.md delete mode 100644 doc/test/running_tests.md delete mode 100644 doc/test/styleguide.md diff --git a/doc/test/php_tests.md b/doc/test/php_tests.md deleted file mode 100644 index b0ab668a6..000000000 --- a/doc/test/php_tests.md +++ /dev/null @@ -1,65 +0,0 @@ -# Writing PHPUnit tests - -## Test path and filename - -The path where you should put your PHPUnit tests should reflect the path in the sourcetree, with test/php/ prepended. So -if you're testing a file library/Icinga/My/File.php the test file should be at test/php/library/Icinga/My/File.php. This -also applies for modules, where the tests are underneath modules/myModule/test/php - -## Example test skeleton - -Let's assume you're testing a class MyClass underneath the MyModule module and the file can be found at -modules/mymodule/library/MyModule/Helper/MyClass.php. - - assertTrue(true, "Asserting that the world didn't end yet"); - } - } - -## Testing Singletons - -When test methods **modify static** class properties (which is the case when using singletons), do not add the PHPUnit -[`@backupStaticAttributes enabled`](http://phpunit.de/manual/3.7/en/appendixes.annotations.html#appendixes.annotations.backupStaticAttributes) -annotation to their [DocBlock](http://www.phpdoc.org/docs/latest/for-users/phpdoc/basic-syntax.html#what-is-a-docblock) -in order to backup and restore static attributes before and after the test execution respectively. Use the setUp() -and tearDown() routines instead to accomplish this task. - - openingHours = CheesecakeFactory::getOpeningHours(); - } - - protected function tearDown() - { - parent::tearDown(); - CheesecakeFactory::setOpeningHours($this->openingHours); - } - - public function testThatInteractsWithStaticAttributes() - { - CheesecakeFactory::setOpeningHours(24); - // ... - } - } - -The reason to avoid using @backupStaticAttributes is the fact that if it is necessary to utilize a -singleton in your *unit* tests you probably want to rethink what you are going to test and because -some tests are using the mock framework [`Mockery`](https://github.com/padraic/mockery) which is -using static class properties to implement its caching mechanics. diff --git a/doc/test/running_tests.md b/doc/test/running_tests.md deleted file mode 100644 index 116911057..000000000 --- a/doc/test/running_tests.md +++ /dev/null @@ -1,88 +0,0 @@ -# Test organization - -## Testfolders - -Tests for the application can be found underneath the test folder: - - test/ - php/ PHPUnit tests for backend code - regression/ PHPUnit regression tests - -The same structure applies for modules, which also contain a toplevel test folder and suitable subtests. When you fix -a bug and write a regression test for it, put it in 'regression' and name it %DESCRIPTION%%TicketNumber% (myBug1234.php) - -## Running tests - -The tests can be run in the specific folder using the runtests script. - -Running PHP tests example: - - cd test/php - ./runtests - -In this case, all application and all module tests will be executed. The testrunners also support additional flags, which -affect they way the test is executed: - - Options: - -h, --help show this help message and exit - -b, --build Enable reporting. - -v, --verbose Be more verbose. - -i PATTERN, --include=PATTERN - Include only specific files/test cases. - -V, --vagrant Run in vagrant VM - -Some tests also support the --exclude method, it's best to use the --help option to see which flags are supported. - - -## Setting up databases - -Despite running most of the tests should work out of the box, a few specific cases require some setup. -At this moment, only database tests require additional setup and expect an icinga_unittest user with an icinga_unittest -database to exist and have rights in your database. - -### The database test procedure - -When testing PostgreSQL and MySQL databases, the test library (normally) executes the following test procedure for every -test case: - -- Log in to the rdbms as the user icinga_unittest with password icinga_unittest -- Use the icinga_unittest database (which must be existing) -- **Drop all tables** in the icinga_unittest database -- Create a new, clean database schema - -If anything goes wrong during this procedure, the test will be skipped (because maybe you don't have a pgsql database, but -want to test mysql, for example). - -### Setting up a test user and database in MySQL - -In MySQL, it's best to create a user icinga_unittest@localhost, a database icinga_unittest and grant all privileges on -this database: - - mysql -u root -p - mysql> CREATE USER `icinga_unittest`@`localhost` IDENTIFIED BY 'icinga_unittest'; - mysql> CREATE DATABASE `icinga_unittest`; - mysql> GRANT ALL PRIVILEGES ON `icinga_unittest`.* TO `icinga_unittest`@`localhost`; - mysql> FLUSH PRIVILEGES; - mysql> quit - -### Setting up a test user and database in PostgreSQL - -In PostgreSQL, you have to modify the pg_hba database if you don't have password authentication set up (which often is -the case). In this setup the icinga_unittest user is set to trust authentication on localhost, which means that no -password is queried when connecting from the local machine: - - sudo su postgres - psql - postgres=# CREATE USER icinga_unittest WITH PASSWORD 'icinga_unittest'; - postgres=# CREATE DATABASE icinga_unittest; - postgres=# \q - bash$ createlang plpgsql icinga; - - -Add the following lines to your pg_hba.conf (etc/postgresql/X.x/main/pg_hba.conf under debian, /var/lib/pgsql/data/pg_hba.conf for Redhat/Fedora) -to enable trust authentication for the icingaweb user when connecting from the localhost. - - local icinga_unittest icinga_unittest trust - host icinga_unittest icinga_unittest 127.0.0.1/32 trust - host icinga_unittest icinga_unittest ::1/128 trust - diff --git a/doc/test/styleguide.md b/doc/test/styleguide.md deleted file mode 100644 index 4186b3375..000000000 --- a/doc/test/styleguide.md +++ /dev/null @@ -1,359 +0,0 @@ -# General testing guidelines - -## The short summary - -This list summarizes what will be described in the next few chapters: - -- You really should write tests for your code -- Think about your what you want to test and how you can assert the behaviour correctly. -- Isolate your tests and start without any assumptions about your test environment (like a specific user existing). The - only excuse here is to assume a correct login if you need a database for testing (but not the tables/content of the database!) -- Don't just test correct behaviour - test border cases and invalid input or assumptions to make sure the application handles - cases that might not occur in a normal test environment -- Use description strings in your assertions, this makes it easy to detect what's going wrong when they fail and it's easier - to follow what *exactly* you are asserting -- Test methods should be one scenario and the method name should describe this scenario. - *testLogin* is bad for example (do you test if the login fails? Or if it is correct? - *testLoginWithCorrectCredentials*, *testLoginWithWrongPassword* is a far better name here -- Your assertions should reflect one test scenario, i.e. don't write one test method that tests if something works **and** - if it correctly detects errors after it works. Write one test to determine the behaviour with correct input and one that - tests the behaviour with invalid input. -- Mock external components and inject them into the class you want to test. If your testsubject is not able to use mocked - dependencies, it's often a design flaw and should be considered as a bug (and be fixed) - - -## What should be tested - -### Writing meaningful tests - -Writing tests doesn't ensure that your code is free of errors, but it can test if, in a specific scenario, the behaviour of -your code is as expected. This means that you have to think about your test scenario before writing a test. This involves -three steps: - -- Determine what you want to test: Which errors can occur, what is the normal input and what are the border cases. -- Define a test scenario: What datasets make sense for the test? -- How do I write my assertions so they are really meaningful about the correctness of the testcase - -Especially the border cases are important, often they lay inside of try/catch blocks or error detection routines. When -looking at your code coverages, these blocks should be covered. - -### Example - -Let's say you have the following function (the examples in this section are to be considered as php-like pseudocode) : - - function isValidName($name) - { - if (hasCorrectFormat($name)) { - return false; - } - - if (nameExistsInDatabase($name)) { - return false; - } - - return true; - } - - -#### Determine what to test: - -At first glance there can be 3 scenarios: - -1. The username is unique and valid -2. The username has the wrong format -3. The username has the correct format, but exists in the database - -But - what happens if the database is down? Should an exception be thrown or should it return false? This case has to be added -to your tests, so you have (at least!) the following scenarios: - -1. The username is unique and valid -2. The username has the wrong format -3. The username has the correct format, but exists in the database -4. The username has the correct format, but access to the database fails. - - -#### Determine meaningful testscenarios - -When it comes to creating your testscenario, we start with the easiest one: Test the wrongly formatted username. This -should be pretty straightforward, as we never reach the code where we need the database: - - function testWrongFormat() - { - assertFalse(isValidName("$$% 1_', "Assert a name with special characters to be considered an invalid name"); - } - -The first and third scenario are more sophisticated (like always, when a database comes into the game). There are two ways -to test this: -- Either you create an empty table on each test, fill them with the users and run the test -- or you Mock the database call with a class that behaves like querying the users and returns true or false in each case - -You **shouldn't** create a static database for all tests and assume this one to be existing - these are decoupled from the -actual test and soon get outdated or difficult to reflect all scenarios. Also it's not considered good practice to create -a precondition your tests have to rely on. In this case the second approach makes sense, as the mock class should be rather simple: - - - function testCorrectUserName() - { - // set no users in the database mock - $db = new UserDatabaseMock(array()); - setupUserDatabaseMock($db); - - assertTrue(isValidName("hans"), "Assert a correctly formatted and unique name to be considered valid"); - } - - function testNonUniqueUserName() - { - // set no users in the database mock - $db = new UserDatabaseMock(array("pete")); - setupUserDatabaseMock($db); - - assertFalse(isValidName("pete"), "Assert a correctly formatted, but existing name to be considered invalid"); - } - -The exception can be tested by providing invalid db credentials when using a real databse or by extending the mock (which -we will do here): - - function testDatabaseError() - { - // set no users in the database mock - $db = new UserDatabaseMock(array()); - $db->setThrowsException(true); - setupUserDatabaseMock($db); - - assertFalse(isValidName("hans"), "Assert a correct, unique user to be considered invalid when the database doesn't work"); - } - -This, of course, depends on how you want the behaviour to be when the db is down: Do you want to log the error and proceed -as usual or do you want the error to bubble up via an exception. - -#### Writing sensible assertions - -It's crucial to write sensible assertions in your test-classes: You can write a perfect test that covers the right scenario, -but don't catch errors because you aren't asking the correct questions. - -- Write assertions that cover your scenario - if you test a correct behaviour don't test what happens if something goes wrong - (this is a seperate scenario) -- While you should try to write redundant assertions it's better to assert more than to have a missing assertion -- A failed assertion means that the implementation is incorrect, other assertions are to be avoided (like testing if a - precondition applies) -- When testing one function, you have to be naive and assume that everything else is bug free, testing whether other parts - worked correctly before testing should be made in a different test. - -## How to test - -Unit tests should only test an isolated, atomic of your code - in theory just one single function - and shouldn't -need much dependency handling. An example for a unittest would be to test the following (hypothetical) class method: - - class UserManager - { - /** - * returns true when a user with this name exists and the - * password for this user is correct - **/ - public function isCorrectPassword($name, $password) - { - // You needn't to know the implementation. - } - } - -### The wrong way - -A unit test for this user could, but should not look like this (we'll explain why): - - use Icinga/MyLibrary/UserManager - - class UserManagerTest extends \PHPUnit_Framework_TestCase - { - /** - * Test whether an user is correctly recognized by the UserManager - * - **/ - public function testUserManager() - { - // Connect to the test database that contains jdoe and jsmith - $mgrConfg = new \Zend_Config(array( - "backend" => "db", - "user" => "dbuser.." - "pass" => - // all the other db credentials - )); - - $mgr = new UserManager($mgrConfig); - - $this->assertTrue($mgr->isCorrectPassword("jdoe", "validpassword")); - $this->assertTrue($mgr->isCorrectPassword("jsmith", "validpassword")); - $this->assertTrue($mgr->isCorrectPassword("jdoe", "nonvalidpassword")); - $this->assertTrue($mgr->isCorrectPassword("jsmith", "nonvalidpassword")); - $this->assertTrue($mgr->isCorrectPassword("hans", "validpasswor")); - } - -This test has a few issues: - -- First, it assert a precondition to apply : A database must exist with the users jdoe and jsmith and the credentials - must match the ones provided in the test -- There are a lot of dependencies in this code, almost the complete Authentication code must exists. Our test - will fail if this code changes or contains bugs, even the isCorrectPassword method is correct. - -### Reducing dependencies - -To avoid these issues, you need to code your classes with testing in mind. Maybe now you're screaming *"Tests shouldn't -affect my code, just test if it is correct!"*, but it's not that easy. Testability should be considered as a quality aspect -of your code, just like commenting, keeping functions coherent, etc. Non-testable code should be considered as a bug, or -at least as a design-flaw. - -One big buzzword in development is now Inversion of Control and Dependency Injection. You can google the details, but in -our case it basically means: Instead of your class (in this case UserManager) setting up it's dependencies (creating an Authentication Manager and -then fetching users from it), the dependencies are given to the class. This has the advantage that you can mock the -dependencies in your testclasses which heavily reduces test-complexity. On the downside this can lead to more complicate -Api's, as you have to know the dependencies of your Object when creating it. Therefore we often allow to provide an -dependency from the outside (when testing), but normally create the dependencies when nothing is provided (normal use). - -In our case we could say that we allow our UserManager to use a provided set of Users instead of fetching it from the -Authmanger: - - class UserManager - { - - public function __construct($config, $authMgr = null) - { - if ($authMgr == null) { - // no Authmanager provided, resolve dependency by yourself - $this->authMgr = new AuthManager($config); - } else { - $this->authMgr = $authMgr; - } - } - } - -It would of course be best to create an Interface like UserSource which the AuthManger implements, but in this example -we trust our Programmer to provide a suitable object. We now can eliminate all the AuthManager dependencies by mocking the -AuthManager (lets dumb it down to just providing an array of users): - - use Icinga/MyLibrary/UserManager - - class AuthManagerMock - { - public $users; - - /** - * Create a new mock classw with the provided users and their credentials - * - * @param array $userPasswordCombinations The users and password combinations to use - **/ - public function __construct(array $userPasswordCombinations) - { - $this->users = $userPasswordCombinations; - } - - public function getUsers() - { - return $this->users; - } - } - - class UserManagerTest extends \PHPUnit_Framework_TestCase - { - /** - * Test whether an user is correctly recognized by the UserManager - * - **/ - public function testUserManager() - { - $authMock = new AuthManagerMock(array( - "jdoe" => "validpassword", - "jsmith" => "validpassword" - )); - $mgrConfg = new \Zend_Config(array(), $authMock); - $mgr = new UserManager($mgrConfig); - - $this->assertTrue($mgr->isCorrectPassword("jdoe", "validpassword")); - $this->assertTrue($mgr->isCorrectPassword("jsmith", "validpassword")); - $this->assertFalse($mgr->isCorrectPassword("jdoe", "nonvalidpassword")); - $this->assertFalse($mgr->isCorrectPassword("jsmith", "nonvalidpassword")); - $this->assertFalse($mgr->isCorrectPassword("hans", "validpassword")); - } - -Ok, we might have more code here than before, but our test is now less like prone to fail: - -- Our test doesn't assume any preconditions to apply, like having a db server with correct users - - - -### Splitting up assertions - -The test is now not that bad, but still has a few issues: - -- If an assert fails, we don't know which one, as the message will be rather generic ("failed asserting that False is True") -- In this case it might be obvious what we test, but if someone sees the class and the assertions, he doesn't know what -assumptions are made - -To fix those issues, we have to split up our big test method in several smaller one and give the testmethod **talking names**. -Also, the assertions should get an error message that will be printed on failure. - - /** - * Testcases for the UserManager class - * - **/ - class UserManagerTest extends \PHPUnit_Framework_TestCase - { - /** - * Creates a new UserManager with a mocked AuthManage - * - * @returns UserManager - **/ - public function getUserManager() - { - $authMock = new AuthManagerMock(array( - "jdoe" => "validpassword", - "jsmith" => "validpassword" - )); - $mgrConfg = new \Zend_Config(array(), $authMock); - return new UserManager($mgrConfig); - } - - /** - * Tests whether correct user/name combinations are considered valid - * - **/ - public function testCorrectUserPasswordCombinations() - { - $mgr = $this->getUserManager(); - $this->assertTrue( - $mgr->isCorrectPassword("jdoe", "validpassword"), - "Asserted that a correct user/password combination is considered valid for jdoe" - ); - $this->assertTrue( - $mgr->isCorrectPassword("jsmith", "validpassword"), - "Asserted that a correct user/password combination is considered valid for jsmith" - ); - } - - /** - * Tests whether invalid names are rejected - * - **/ - public function testInvalidUsernameRecognition() - { - $mgr = $this->getUserManager(); - $this->assertFalse( - $mgr->isCorrectPassword("hans", "validpassword"), - "Asserted a non-existing user to be be considered invalid" - ); - } - - /** - * Tests whether invalid passwords for existing users are rejected - * - **/ - public function testInvalidPasswordRecognition() - { - $mgr = $this->getUserManager(); - $this->assertFalse( - $mgr->isCorrectPassword("jsmith", "nonvalidpassword"), - "Asserted that an invalid password for an existing user is considered invalid" - ); - } - } - -Now if something fails, we now see what has been tested via the testmethod and what caused the test to fail in the -assertion error message. You could also leave the comments and everybody knows what you are doing.