From 34e322db01f7bedc1a801a8c3049f6afd5805fb5 Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 21 Apr 2020 23:25:16 +1200 Subject: [PATCH] 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 --- tests/phpunit/CiviTest/CiviUnitTestCase.php | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 9c5498db65..10cb9dab55 100644 --- 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 === ',' ? '.' : ',')); } /** -- 2.25.1