Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
| 1 | <?php |
||
| 33 | class UsersModuleTest extends PHPUnit_Framework_TestCase |
||
| 34 | { |
||
| 35 | /** @var \SURFnet\VPN\Common\Http\Service */ |
||
| 36 | private $service; |
||
| 37 | |||
| 38 | public function setUp() |
||
| 83 | |||
| 84 | public function testListUsers() |
||
| 85 | { |
||
| 86 | $this->assertSame( |
||
| 87 | [ |
||
| 88 | [ |
||
| 89 | 'user_id' => 'foo', |
||
| 90 | 'is_disabled' => false, |
||
| 91 | 'two_factor' => false, |
||
| 92 | ], |
||
| 93 | [ |
||
| 94 | 'user_id' => 'bar', |
||
| 95 | 'is_disabled' => true, |
||
| 96 | 'two_factor' => true, |
||
| 97 | ], |
||
| 98 | [ |
||
| 99 | 'user_id' => 'baz', |
||
| 100 | 'is_disabled' => false, |
||
| 101 | 'two_factor' => true, |
||
| 102 | ], |
||
| 103 | ], |
||
| 104 | $this->makeRequest( |
||
| 105 | ['vpn-admin-portal', 'bbccdd'], |
||
| 106 | 'GET', |
||
| 107 | 'user_list', |
||
| 108 | ['profile_id' => 'internet'], |
||
| 109 | [] |
||
| 110 | ) |
||
| 111 | ); |
||
| 112 | } |
||
| 113 | |||
| 114 | View Code Duplication | public function testSetTotpSecret() |
|
| 115 | { |
||
| 116 | $otp = new Otp(); |
||
| 117 | $totpSecret = 'MM7TTLHPA7WZOJFB'; |
||
| 118 | $totpKey = $otp->totp(Base32::decode($totpSecret)); |
||
| 119 | |||
| 120 | $this->assertTrue( |
||
| 121 | $this->makeRequest( |
||
| 122 | ['vpn-user-portal', 'aabbcc'], |
||
| 123 | 'POST', |
||
| 124 | 'set_totp_secret', |
||
| 125 | [], |
||
| 126 | [ |
||
| 127 | 'user_id' => 'foo', |
||
| 128 | 'totp_secret' => $totpSecret, |
||
| 129 | 'totp_key' => $totpKey, |
||
| 130 | ] |
||
| 131 | ) |
||
| 132 | ); |
||
| 133 | } |
||
| 134 | |||
| 135 | View Code Duplication | public function testVerifyOtpKey() |
|
| 136 | { |
||
| 137 | $otp = new Otp(); |
||
| 138 | $totpSecret = 'CN2XAL23SIFTDFXZ'; |
||
| 139 | $totpKey = $otp->totp(Base32::decode($totpSecret)); |
||
| 140 | |||
| 141 | $this->assertTrue( |
||
| 142 | $this->makeRequest( |
||
| 143 | ['vpn-user-portal', 'aabbcc'], |
||
| 144 | 'POST', |
||
| 145 | 'verify_totp_key', |
||
| 146 | [], |
||
| 147 | [ |
||
| 148 | 'user_id' => 'bar', |
||
| 149 | 'totp_key' => $totpKey, |
||
| 150 | ] |
||
| 151 | ) |
||
| 152 | ); |
||
| 153 | } |
||
| 154 | |||
| 155 | public function testVerifyOtpKeyWrong() |
||
| 156 | { |
||
| 157 | // in theory this totp_key, 123456 could be correct at one point in |
||
| 158 | // time... then this test will fail! |
||
| 159 | $this->assertSame( |
||
| 160 | [ |
||
| 161 | 'ok' => false, |
||
| 162 | 'error' => 'invalid TOTP key', |
||
| 163 | ], |
||
| 164 | $this->makeRequest( |
||
| 165 | ['vpn-user-portal', 'aabbcc'], |
||
| 166 | 'POST', |
||
| 167 | 'verify_totp_key', |
||
| 168 | [], |
||
| 169 | [ |
||
| 170 | 'user_id' => 'bar', |
||
| 171 | 'totp_key' => '123456', |
||
| 172 | ] |
||
| 173 | ) |
||
| 174 | ); |
||
| 175 | } |
||
| 176 | |||
| 177 | public function testVerifyOtpKeyReplay() |
||
| 199 | |||
| 200 | public function testHasTotpSecret() |
||
| 214 | |||
| 215 | View Code Duplication | public function testDeleteTotpSecret() |
|
| 229 | |||
| 230 | public function testSetVootToken() |
||
| 245 | |||
| 246 | View Code Duplication | public function testDeleteVootToken() |
|
| 260 | |||
| 261 | View Code Duplication | public function testDisableUser() |
|
| 275 | |||
| 276 | View Code Duplication | public function testEnableUser() |
|
| 290 | |||
| 291 | View Code Duplication | public function testDeleteUser() |
|
| 305 | |||
| 306 | public function testUserGroups() |
||
| 330 | |||
| 331 | View Code Duplication | private function makeRequest(array $basicAuth, $requestMethod, $pathInfo, array $getData = [], array $postData = []) |
|
| 361 | } |
||
| 362 |
Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: