From: Eileen McNaughton <emcnaughton@wikimedia.org>
Date: Tue, 1 Jun 2021 03:30:56 +0000 (+1200)
Subject: Merge pull request #20456 from eileenmcnaughton/group2
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=3f2768637f6b0571983ab33933af6c9c1249abdd;hp=cb09fd1c5e65eabf2dc915fe5d6053b0c377731f;p=civicrm-core.git

Merge pull request #20456 from eileenmcnaughton/group2

[Ref] remove never-passed param
---

diff --git a/CRM/Contact/BAO/GroupContactCache.php b/CRM/Contact/BAO/GroupContactCache.php
index 789435e02e..68f3d0b39f 100644
--- a/CRM/Contact/BAO/GroupContactCache.php
+++ b/CRM/Contact/BAO/GroupContactCache.php
@@ -67,7 +67,7 @@ class CRM_Contact_BAO_GroupContactCache extends CRM_Contact_DAO_GroupContactCach
    * @return string
    *   the sql query which lists the groups that need to be refreshed
    */
-  public static function groupRefreshedClause($groupIDClause = NULL, $includeHiddenGroups = FALSE): string {
+  protected static function groupRefreshedClause($groupIDClause = NULL, $includeHiddenGroups = FALSE): string {
     $smartGroupCacheTimeoutDateTime = self::getCacheInvalidDateTime();
 
     $query = "
@@ -162,7 +162,7 @@ AND (
    * @param array $groupID
    * @param array $values
    */
-  public static function store($groupID, &$values) {
+  protected static function store($groupID, &$values) {
     $processed = FALSE;
 
     // sort the values so we put group IDs in front and hence optimize
@@ -416,7 +416,7 @@ WHERE  id IN ( $groupIDs )
    *
    * @return int
    */
-  public static function smartGroupCacheTimeout() {
+  protected static function smartGroupCacheTimeout() {
     $config = CRM_Core_Config::singleton();
 
     if (
diff --git a/tests/phpunit/CRM/Report/Form/TestCaseTest.php b/tests/phpunit/CRM/Report/Form/TestCaseTest.php
index 419ce4cb16..b86a9631b4 100644
--- a/tests/phpunit/CRM/Report/Form/TestCaseTest.php
+++ b/tests/phpunit/CRM/Report/Form/TestCaseTest.php
@@ -9,8 +9,6 @@
  +--------------------------------------------------------------------+
  */
 
-require_once 'CiviTest/CiviReportTestCase.php';
-
 use Civi\Test\Invasive;
 
 /**
@@ -32,10 +30,19 @@ class CRM_Report_Form_TestCaseTest extends CiviReportTestCase {
     'civicrm_contribution',
   ];
 
+  /**
+   * Financial data used in these tests is invalid - do not validate.
+   *
+   * Note ideally it would be fixed and we would always use valid data.
+   *
+   * @var bool
+   */
+  protected $isValidateFinancialsOnPostAssert = FALSE;
+
   /**
    * @return array
    */
-  public function dataProvider() {
+  public function dataProvider(): array {
     $testCaseA = [
       'CRM_Report_Form_Contribute_Detail',
       [
@@ -69,7 +76,7 @@ class CRM_Report_Form_TestCaseTest extends CiviReportTestCase {
   /**
    * @return array
    */
-  public function badDataProvider() {
+  public function badDataProvider(): array {
     return [
       // This test-case is bad because the dataset-ascii.sql does not match the
       // report.csv (due to differences in international chars)
@@ -115,6 +122,9 @@ class CRM_Report_Form_TestCaseTest extends CiviReportTestCase {
     ];
   }
 
+  /**
+   * @throws \CRM_Core_Exception
+   */
   public function setUp(): void {
     parent::setUp();
     $this->quickCleanup($this->_tablesToTruncate);
@@ -128,54 +138,41 @@ class CRM_Report_Form_TestCaseTest extends CiviReportTestCase {
    * @param $expectedOutputCsvFile
    * @throws \Exception
    */
-  public function testReportOutput($reportClass, $inputParams, $dataSet, $expectedOutputCsvFile) {
+  public function testReportOutput($reportClass, $inputParams, $dataSet, $expectedOutputCsvFile): void {
     $config = CRM_Core_Config::singleton();
-    CRM_Utils_File::sourceSQLFile($config->dsn, dirname(__FILE__) . "/{$dataSet}");
+    CRM_Utils_File::sourceSQLFile($config->dsn, __DIR__ . "/$dataSet");
 
     $reportCsvFile = $this->getReportOutputAsCsv($reportClass, $inputParams);
     $reportCsvArray = $this->getArrayFromCsv($reportCsvFile);
 
-    $expectedOutputCsvArray = $this->getArrayFromCsv(dirname(__FILE__) . "/{$expectedOutputCsvFile}");
+    $expectedOutputCsvArray = $this->getArrayFromCsv(__DIR__ . "/$expectedOutputCsvFile");
     $this->assertCsvArraysEqual($expectedOutputCsvArray, $reportCsvArray);
   }
 
   /**
    * @dataProvider badDataProvider
+   *
    * @param $reportClass
    * @param $inputParams
    * @param $dataSet
    * @param $expectedOutputCsvFile
-   * @throws \Exception
+   *
+   * @throws \CRM_Core_Exception
    */
-  public function testBadReportOutput($reportClass, $inputParams, $dataSet, $expectedOutputCsvFile) {
-    $config = CRM_Core_Config::singleton();
-    CRM_Utils_File::sourceSQLFile($config->dsn, dirname(__FILE__) . "/{$dataSet}");
+  public function testBadReportOutput($reportClass, $inputParams, $dataSet, $expectedOutputCsvFile): void {
+    CRM_Utils_File::sourceSQLFile(CIVICRM_DSN, __DIR__ . "/$dataSet");
 
     $reportCsvFile = $this->getReportOutputAsCsv($reportClass, $inputParams);
     $reportCsvArray = $this->getArrayFromCsv($reportCsvFile);
 
-    $expectedOutputCsvArray = $this->getArrayFromCsv(dirname(__FILE__) . "/{$expectedOutputCsvFile}");
-    try {
-      $this->assertCsvArraysEqual($expectedOutputCsvArray, $reportCsvArray);
-      // @todo But...doesn't this mean this test can't ever notify you of a
-      // fail? I think what you could do instead is right here in the try
-      // section throw something that doesn't get caught, since then that means
-      // the previous line passed and so the arrays ARE equal, which means
-      // something is wrong. Or just don't use assertCsvArraysEqual and
-      // explicity compare that they are NOT equal.
-    }
-    catch (PHPUnit\Framework\AssertionFailedError $e) {
-      /* OK */
-    }
-    catch (PHPUnit_Framework_AssertionFailedError $e) {
-      /* OK */
-    }
+    $expectedOutputCsvArray = $this->getArrayFromCsv(__DIR__ . "/$expectedOutputCsvFile");
+    $this->assertNotEquals($expectedOutputCsvArray[1], $reportCsvArray[1]);
   }
 
   /**
    * Test processReportMode() Function in Reports
    */
-  public function testOutputMode() {
+  public function testOutputMode(): void {
     $reportForm = new CRM_Report_Form();
 
     Invasive::set([$reportForm, '_params'], ['groups' => 4]);
diff --git a/tests/phpunit/CiviTest/CiviReportTestCase.php b/tests/phpunit/CiviTest/CiviReportTestCase.php
index d3f1e4adfd..687f808524 100644
--- a/tests/phpunit/CiviTest/CiviReportTestCase.php
+++ b/tests/phpunit/CiviTest/CiviReportTestCase.php
@@ -29,7 +29,7 @@ class CiviReportTestCase extends CiviUnitTestCase {
    * @param array $inputParams
    *
    * @return string
-   * @throws Exception
+   * @throws \CRM_Core_Exception
    */
   public function getReportOutputAsCsv($reportClass, $inputParams) {
 
@@ -46,9 +46,9 @@ class CiviReportTestCase extends CiviUnitTestCase {
    * @param array $inputParams
    *
    * @return CRM_Report_Form
-   * @throws Exception
+   * @throws \CRM_Core_Exception
    */
-  public function getReportObject($reportClass, $inputParams) {
+  public function getReportObject(string $reportClass, array $inputParams): CRM_Report_Form {
     $config = CRM_Core_Config::singleton();
     $config->keyDisable = TRUE;
     $controller = new CRM_Core_Controller_Simple($reportClass, ts('some title'));
@@ -81,7 +81,7 @@ class CiviReportTestCase extends CiviUnitTestCase {
       $reportObj->storeResultSet();
       $reportObj->buildForm();
     }
-    catch (Exception $e) {
+    catch (CRM_Core_Exception $e) {
       // print_r($e->getCause()->getUserInfo());
       CRM_Utils_GlobalStack::singleton()->pop();
       throw $e;
@@ -121,10 +121,8 @@ class CiviReportTestCase extends CiviUnitTestCase {
       . "\n===== ACTUAL DATA ====\n"
       . $this->flattenCsvArray($actualCsvArray);
 
-    $this->assertEquals(
-      count($actualCsvArray),
-      count($expectedCsvArray),
-      'Arrays have different number of rows; in line ' . __LINE__ . '; data: ' . $flatData
+    $this->assertCount(
+      count($actualCsvArray), $expectedCsvArray, 'Arrays have different number of rows; data: ' . $flatData
     );
 
     foreach ($actualCsvArray as $intKey => $strVal) {