From 3e823c3cf7488792574f3d2c65c310394ee140ff Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 8 Feb 2023 15:16:36 +1300 Subject: [PATCH] NFC cleanup in test class --- .../CRM/Core/BAO/SchemaHandlerTest.php | 60 +++++++------- tests/phpunit/CRM/Dedupe/DedupeFinderTest.php | 79 +++++++++---------- 2 files changed, 66 insertions(+), 73 deletions(-) diff --git a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php index ed28228217..78fa3f1bc1 100644 --- a/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php +++ b/tests/phpunit/CRM/Core/BAO/SchemaHandlerTest.php @@ -21,8 +21,6 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Ensure any removed indices are put back. - * - * @throws \CRM_Core_Exception */ public function tearDown(): void { parent::tearDown(); @@ -35,7 +33,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { * We want to be sure it creates an index and exits gracefully if the index * already exists. */ - public function testCreateIndex() { + public function testCreateIndex(): void { $tables = ['civicrm_uf_join' => ['weight']]; CRM_Core_BAO_SchemaHandler::createIndexes($tables); CRM_Core_BAO_SchemaHandler::createIndexes($tables); @@ -54,9 +52,9 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Test CRM_Core_BAO_SchemaHandler::getIndexes() function */ - public function testGetIndexes() { + public function testGetIndexes(): void { $indexes = CRM_Core_BAO_SchemaHandler::getIndexes(['civicrm_contact']); - $this->assertTrue(array_key_exists('index_contact_type', $indexes['civicrm_contact'])); + $this->assertArrayHasKey('index_contact_type', $indexes['civicrm_contact']); } /** @@ -65,7 +63,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { * We want to be sure it creates an index and exits gracefully if the index * already exists. */ - public function testCombinedIndex() { + public function testCombinedIndex(): void { $tables = ['civicrm_uf_join' => ['weight']]; CRM_Core_BAO_SchemaHandler::createIndexes($tables); @@ -94,28 +92,28 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Test the drop index if exists function for a non-existent index. */ - public function testCheckIndexNotExists() { + public function testCheckIndexNotExists(): void { $this->assertFalse(CRM_Core_BAO_SchemaHandler::checkIfIndexExists('civicrm_contact', 'magic_button')); } /** * Test the drop index if exists function for a non-existent index. */ - public function testCheckIndexExists() { + public function testCheckIndexExists(): void { $this->assertTrue(CRM_Core_BAO_SchemaHandler::checkIfIndexExists('civicrm_contact', 'index_hash')); } /** * Test the drop index if exists function for a non-existent index. */ - public function testDropIndexNoneExists() { + public function testDropIndexNoneExists(): void { CRM_Core_BAO_SchemaHandler::dropIndexIfExists('civicrm_contact', 'magic_button'); } /** * Test the drop index if exists function. */ - public function testDropIndexExists() { + public function testDropIndexExists(): void { CRM_Core_BAO_SchemaHandler::dropIndexIfExists('civicrm_contact', 'index_hash'); $this->assertFalse(CRM_Core_BAO_SchemaHandler::checkIfIndexExists('civicrm_contact', 'index_hash')); @@ -126,7 +124,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * @return array */ - public function columnTests() { + public function columnTests(): array { $columns = []; $columns[] = ['civicrm_contribution', 'total_amount']; $columns[] = ['civicrm_contact', 'first_name']; @@ -135,12 +133,12 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { } /** - * @param $tableName - * @param $columnName + * @param string $tableName + * @param string $columnName * * @dataProvider columnTests */ - public function testCheckIfColumnExists($tableName, $columnName) { + public function testCheckIfColumnExists(string $tableName, string $columnName): void { if ($columnName === 'xxxx') { $this->assertFalse(CRM_Core_BAO_SchemaHandler::checkIfFieldExists($tableName, $columnName)); } @@ -152,7 +150,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * @return array */ - public function foreignKeyTests() { + public function foreignKeyTests(): array { $keys = []; $keys[] = ['civicrm_mailing_recipients', 'FK_civicrm_mailing_recipients_email_id']; $keys[] = ['civicrm_mailing_recipients', 'FK_civicrm_mailing_recipients_id']; @@ -166,8 +164,10 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { * * @param string $tableName * @param string $key + * + * @noinspection PhpUnusedParameterInspection */ - public function testSafeDropForeignKey($tableName, $key) { + public function testSafeDropForeignKey(string $tableName, string $key): void { if ($key === 'FK_civicrm_mailing_recipients_id') { $this->assertFalse(CRM_Core_BAO_SchemaHandler::safeRemoveFK('civicrm_mailing_recipients', $key)); } @@ -179,7 +179,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Check there are no missing indices */ - public function testGetMissingIndices() { + public function testGetMissingIndices(): void { $missingIndices = CRM_Core_BAO_SchemaHandler::getMissingIndices(); $this->assertEmpty($missingIndices); } @@ -187,7 +187,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Test that missing indices are correctly created */ - public function testCreateMissingIndices() { + public function testCreateMissingIndices(): void { $indices = [ 'test_table' => [ 'test_index1' => [ @@ -222,11 +222,9 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { } /** - * Check there are no missing indices - * - * @throws \CRM_Core_Exception + * Check there are no missing indices. */ - public function testReconcileMissingIndices() { + public function testReconcileMissingIndices(): void { CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_contact DROP INDEX index_sort_name'); $missingIndices = CRM_Core_BAO_SchemaHandler::getMissingIndices(); // Check the api also retrieves them. @@ -248,11 +246,9 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { } /** - * Check there are no missing indices - * - * @throws \CRM_Core_Exception + * Check there are no missing indices. */ - public function testGetMissingIndicesWithTableFilter() { + public function testGetMissingIndicesWithTableFilter(): void { CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_contact DROP INDEX index_sort_name'); CRM_Core_DAO::executeQuery('ALTER TABLE civicrm_contribution DROP INDEX index_total_amount_receive_date'); $missingIndices = $this->callAPISuccess('System', 'getmissingindices', [])['values']; @@ -285,7 +281,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Check for partial indices */ - public function testPartialIndices() { + public function testPartialIndices(): void { $tables = [ 'index_all' => 'civicrm_prevnext_cache', 'UI_entity_id_entity_table_tag_id' => 'civicrm_entity_tag', @@ -306,7 +302,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { //Check if both indices are deleted. $indices = CRM_Core_BAO_SchemaHandler::getIndexes($tables); foreach ($tables as $index => $tableName) { - $this->assertFalse(array_key_exists($index, $indices[$tableName])); + $this->assertArrayNotHasKey($index, $indices[$tableName]); } //Drop false index and create again. CRM_Core_BAO_SchemaHandler::createMissingIndices($missingIndices); @@ -318,7 +314,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Test index signatures are added correctly */ - public function testAddIndexSignatures() { + public function testAddIndexSignatures(): void { $indices = [ 'one' => [ 'field' => ['id', 'name(3)'], @@ -329,8 +325,8 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { ], ]; CRM_Core_BAO_SchemaHandler::addIndexSignature('my_table', $indices); - $this->assertEquals($indices['one']['sig'], 'my_table::1::id::name(3)'); - $this->assertEquals($indices['two']['sig'], 'my_table::0::title'); + $this->assertEquals('my_table::1::id::name(3)', $indices['one']['sig']); + $this->assertEquals('my_table::0::title', $indices['two']['sig']); } /** @@ -374,7 +370,7 @@ class CRM_Core_BAO_SchemaHandlerTest extends CiviUnitTestCase { /** * Tests the function that generates sql to modify fields. */ - public function testBuildFieldChangeSql() { + public function testBuildFieldChangeSql(): void { $params = [ 'table_name' => 'civicrm_contact', 'operation' => 'add', diff --git a/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php b/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php index c1d1676750..73a6743296 100644 --- a/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php +++ b/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php @@ -23,8 +23,6 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { /** * Clean up after the test. - * - * @throws \CRM_Core_Exception */ public function tearDown(): void { @@ -46,7 +44,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \Exception */ - public function testUnsupervisedDupes() { + public function testUnsupervisedDupes(): void { // make dupe checks based on following contact sets: // FIRST - LAST - EMAIL // --------------------------------- @@ -62,7 +60,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { $ruleGroup = $this->callAPISuccessGetSingle('RuleGroup', ['is_reserved' => 1, 'contact_type' => 'Individual', 'used' => 'Unsupervised']); $foundDupes = CRM_Dedupe_Finder::dupesInGroup($ruleGroup['id'], $this->groupID); - $this->assertEquals(count($foundDupes), 3, 'Check Individual-Fuzzy dupe rule for dupesInGroup().'); + $this->assertCount(3, $foundDupes, 'Check Individual-Fuzzy dupe rule for dupesInGroup().'); } /** @@ -70,7 +68,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testUnsupervisedWithTwoEmailFields() { + public function testUnsupervisedWithTwoEmailFields(): void { $this->setupForGroupDedupe(); $emails = [ ['hood@example.com', ''], @@ -85,7 +83,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { ]; $dedupeParams = CRM_Dedupe_Finder::formatParams($fields, 'Individual'); $dedupeResults = CRM_Dedupe_Finder::dupesByParams($dedupeParams, 'Individual'); - $this->assertEquals(count($dedupeResults), 1); + $this->assertCount(1, $dedupeResults); } } @@ -96,7 +94,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testCustomRule() { + public function testCustomRule(): void { $this->setupForGroupDedupe(); $ruleGroup = $this->createRuleGroup(); @@ -109,7 +107,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { ]); } $foundDupes = CRM_Dedupe_Finder::dupesInGroup($ruleGroup['id'], $this->groupID); - $this->assertEquals(count($foundDupes), 4); + $this->assertCount(4, $foundDupes); CRM_Dedupe_Finder::dupes($ruleGroup['id']); } @@ -119,12 +117,12 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testCustomRuleCustomFields() { + public function testCustomRuleCustomFields(): void { $this->setupForGroupDedupe(); //Create custom group with fields of all types to test. - $customGroup = $this->createCustomGroup(); + $this->createCustomGroup(); $customGroupID = $this->ids['CustomGroup']['Custom Group']; $ids = &$this->ids['CustomField']; @@ -136,11 +134,11 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { foreach ($this->ids['CustomField'] as $key => $field_id) { switch ($key) { case 'string': - $params["custom_{$field_id}"] = "text"; + $params["custom_{$field_id}"] = 'text'; break; case 'date': - $params["custom_{$field_id}"] = "20220511"; + $params["custom_{$field_id}"] = '20220511'; break; case 'int': @@ -152,13 +150,13 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { $count = 0; foreach ($this->contactIDs as $contact_id) { // Update the text custom fields for duplicate contact - foreach ($this->ids['CustomField'] as $key => $field_id) { + foreach ($this->ids['CustomField'] as $ignored) { $this->callAPISuccess('Contact', 'create', array_merge([ 'id' => $contact_id, ], $params)); } $count++; - if ($count == 2) { + if ($count === 2) { break; } } @@ -205,13 +203,13 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testCustomRuleCustomFieldsSubtypes() { + public function testCustomRuleCustomFieldsSubtypes(): void { $this->setupForGroupDedupe(); - //Create custom group with fields of all types to test. - $contactType = $this->callAPISuccess('ContactType', 'create', ['name' => 'Big Bank', 'label' => 'biggee', 'parent_id' => 'Individual']); - $customGroup = $this->createCustomGroup(['extends' => 'Individual', 'extends_entity_column_value' => ['Big_Bank']]); + // Create custom group with fields of all types to test. + $this->callAPISuccess('ContactType', 'create', ['name' => 'Big Bank', 'label' => 'biggie', 'parent_id' => 'Individual']); + $this->createCustomGroup(['extends' => 'Individual', 'extends_entity_column_value' => ['Big_Bank']]); $customGroupID = $this->ids['CustomGroup']['Custom Group']; $ids = &$this->ids['CustomField']; @@ -223,11 +221,11 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { foreach ($this->ids['CustomField'] as $key => $field_id) { switch ($key) { case 'string': - $params["custom_{$field_id}"] = "text"; + $params["custom_{$field_id}"] = 'text'; break; case 'date': - $params["custom_{$field_id}"] = "20220511"; + $params["custom_{$field_id}"] = '20220511'; break; case 'int': @@ -246,13 +244,13 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { $count = 0; foreach ($this->contactIDs as $contact_id) { // Update the text custom fields for duplicate contact - foreach ($this->ids['CustomField'] as $key => $field_id) { + foreach ($this->ids['CustomField'] as $ignored) { $this->callAPISuccess('Contact', 'create', array_merge([ 'id' => $contact_id, ], $params)); } $count++; - if ($count == 2) { + if ($count === 2) { break; } } @@ -299,7 +297,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testCustomRuleCustomDateField() { + public function testCustomRuleCustomDateField(): void { $ruleGroup = $this->createRuleGroup(); $this->createCustomGroupWithFieldOfType([], 'date'); @@ -318,7 +316,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testCustomRuleWithAddress() { + public function testCustomRuleWithAddress(): void { $this->setupForGroupDedupe(); $ruleGroup = $this->callAPISuccess('RuleGroup', 'create', [ @@ -329,9 +327,9 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { 'title' => 'TestRule', 'is_reserved' => 0, ]); - $rules = []; + foreach (['postal_code'] as $field) { - $rules[$field] = $this->callAPISuccess('Rule', 'create', [ + $this->callAPISuccess('Rule', 'create', [ 'dedupe_rule_group_id' => $ruleGroup['id'], 'rule_table' => 'civicrm_address', 'rule_weight' => 10, @@ -339,7 +337,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { ]); } $foundDupes = CRM_Dedupe_Finder::dupesInGroup($ruleGroup['id'], $this->groupID); - $this->assertEquals(count($foundDupes), 1); + $this->assertCount(1, $foundDupes); CRM_Dedupe_Finder::dupes($ruleGroup['id']); } @@ -349,7 +347,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testRuleThreeContactFieldsEqualWeightWIthThresholdtheTotalSumOfAllWeight() { + public function testRuleThreeContactFieldsEqualWeightWithThresholdTheTotalSumOfAllWeight(): void { $this->setupForGroupDedupe(); $ruleGroup = $this->callAPISuccess('RuleGroup', 'create', [ @@ -378,7 +376,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testInclusiveRule() { + public function testInclusiveRule(): void { $this->setupForGroupDedupe(); $ruleGroup = $this->createRuleGroup(); @@ -400,7 +398,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \Exception */ - public function testSupervisedDupes() { + public function testSupervisedDupes(): void { $this->setupForGroupDedupe(); $ruleGroup = $this->callAPISuccessGetSingle('RuleGroup', ['is_reserved' => 1, 'contact_type' => 'Individual', 'used' => 'Supervised']); $foundDupes = CRM_Dedupe_Finder::dupesInGroup($ruleGroup['id'], $this->groupID); @@ -410,7 +408,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { // will - dale - will@example.com // will - dale - will@example.com // so 1 pair for - first + last + mail - $this->assertEquals(count($foundDupes), 1, 'Check Individual-Fuzzy dupe rule for dupesInGroup().'); + $this->assertCount(1, $foundDupes, 'Check Individual-Fuzzy dupe rule for dupesInGroup().'); } /** @@ -418,7 +416,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * @throws \CRM_Core_Exception */ - public function testDupesByParams() { + public function testDupesByParams(): void { // make dupe checks based on based on following contact sets: // FIRST - LAST - EMAIL // --------------------------------- @@ -492,8 +490,8 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { $contactIds[$count++] = $contact['id']; } - // verify that all contacts have been created separately - $this->assertEquals(count($contactIds), 7, 'Check for number of contacts.'); + // Verify that all contacts have been created separately. + $this->assertCount(7, $contactIds, 'Check for number of contacts.'); $fields = [ 'first_name' => 'robin', @@ -504,7 +502,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { $ids = CRM_Contact_BAO_Contact::getDuplicateContacts($fields, 'Individual', 'General', [], TRUE, NULL, ['event_id' => 1]); // Check with default Individual-General rule - $this->assertEquals(count($ids), 2, 'Check Individual-General rule for dupesByParams().'); + $this->assertCount(2, $ids, 'Check Individual-General rule for dupesByParams().'); // delete all created contacts foreach ($contactIds as $contactId) { @@ -517,6 +515,7 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { * * Locks in expected params * + * @noinspection PhpParameterByRefIsNotUsedAsReferenceInspection */ public function hook_civicrm_findDuplicates($dedupeParams, &$dedupeResults, $contextParams) { $expectedDedupeParams = [ @@ -546,11 +545,9 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { } /** - * Set up a group of dedupable contacts. - * - * @throws \CRM_Core_Exception + * Set up a group of dedupe-able contacts. */ - protected function setupForGroupDedupe() { + protected function setupForGroupDedupe(): void { $params = [ 'name' => 'Dupe Group', 'title' => 'New Test Dupe Group', @@ -623,8 +620,8 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { $this->callAPISuccess('group_contact', 'create', $grpParams); } - // verify that all contacts have been created separately - $this->assertEquals(count($this->contactIDs), 7, 'Check for number of contacts.'); + // Verify that all contacts have been created separately. + $this->assertCount(7, $this->contactIDs, 'Check for number of contacts.'); } } -- 2.25.1