Comment clarification in test class

I just updated the comments on this helper to clarify the limitations of the function & the
fact that it should not be our only way to test thousand separators.

I was noticing perfect was becoming the enemy of the good here. The function was marked as deprecated
because it doesn't cover all scenarios - but the upshot was that we stopped increasing out
thousand separator testing. In fact we need lots of form tests to do some testing of
the separators and a very small number to test more variants - this latter has been added
& the comments point to the need for more without going as far as deprecating

diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php
index 9c5498db65efeb6bd70dacc37ed305dc3c137390..10cb9dab5572e8a17e6d8c804989febaeb6a805d 100644 (file)
--- a/tests/phpunit/CiviTest/CiviUnitTestCase.php
+++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php
@@ -3271,19 +3271,18 @@ VALUES
   /**
    * Set the separators for thousands and decimal points.
    *
-   * Use setMonetaryDecimalPoint and setMonetaryThousandSeparator instead
+   * Note that this only covers some common scenarios.
+   *
+   * It does not cater for a situation where the thousand separator is a [space]
+   * Latter is the Norwegian localization. At least some tests need to
+   * use setMonetaryDecimalPoint and setMonetaryThousandSeparator directly
+   * to provide broader coverage.
    *
    * @param string $thousandSeparator
-   * @deprecated
    */
   protected function setCurrencySeparators($thousandSeparator) {
-    // Jaap Jansma: I do think the code below is a bit useless. It does an assumption
-    // that when the thousand separator is a comma, the decimal is point. It
-    // does not cater for a situation where the thousand separator is a [space]
-    // Latter is the Norwegian localization.
     Civi::settings()->set('monetaryThousandSeparator', $thousandSeparator);
-    Civi::settings()
-      ->set('monetaryDecimalPoint', ($thousandSeparator === ',' ? '.' : ','));
+    Civi::settings()->set('monetaryDecimalPoint', ($thousandSeparator === ',' ? '.' : ','));
   }
 
   /**