Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix language strings plural suffixes unit test failing when executing JLanguageTest.php alone #35026

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Aug 2, 2021

Pull Request for Issue # .

Summary of Changes

This pull request (PR) fixes unit tests for the "getPluralSuffixes" function of JText currently failing when executed locally (at least on Linux).

No idea why they don't fail in drone CI.

The failure is caused by file "tests/unit/suites/libraries/joomla/language/data/language/en-GB/en-GB.localise.php" having not been adapted with PR #28763 .

In addition, this PR adds a missing test case for the "more than 1" case.

Testing Instructions

  1. On a git clone, checkout and if necessary update the staging branch so it's up to date with the staging branch here.
  2. Open a command window and change directory to the root folder of that git clone.
  3. Run composer install.
  4. Run ./libraries/vendor/phpunit/phpunit/phpunit ./tests/unit/suites/libraries/joomla/language/JLanguageTest.php
    Result: See section "Actual result BEFORE applying this Pull Request" below.
  5. Revert all local changes by running these 2 git commands:
    git clean -d -x -f
    git checkout .
  6. Apply the changes from this PR.
  7. Repeat steps 2 to 4.
    Result: See section "Expected result AFTER applying this Pull Request" below.

Actual result BEFORE applying this Pull Request

richard@vmubu01:~/lamp/public_html/joomla-cms$ ./libraries/vendor/phpunit/phpunit/phpunit ./tests/unit/suites/libraries/joomla/language/JLanguageTest.php
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
Warning:	The Xdebug extension is not loaded
		No code coverage will be generated.

......F.....................................

Time: 113 ms, Memory: 10.00MB

There was 1 failure:

1) JLanguageTest::testGetPluralSuffixes
Line: 286
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'ONE'
-    1 => '1'
+    0 => '1'
 )

/home/richard/lamp/public_html/joomla-cms/tests/unit/suites/libraries/joomla/language/JLanguageTest.php:286

FAILURES!
Tests: 44, Assertions: 119, Failures: 1.

Expected result AFTER applying this Pull Request

richard@vmubu01:~/lamp/public_html/joomla-cms$ ./libraries/vendor/phpunit/phpunit/phpunit ./tests/unit/suites/libraries/joomla/language/JLanguageTest.php
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.
Warning:	The Xdebug extension is not loaded
		No code coverage will be generated.

............................................

Time: 118 ms, Memory: 8.00MB

OK (44 tests, 120 assertions)

Documentation Changes Required

None.

@richard67 richard67 changed the title Fix language strings plural suffixes unit test Fix language strings plural suffixes unit test failing when executed locally Aug 2, 2021
@richard67 richard67 changed the title Fix language strings plural suffixes unit test failing when executed locally Fix language strings plural suffixes unit test failing when executing JLanguageTest.php alone Aug 2, 2021
@Fedik
Copy link
Member

Fedik commented Aug 2, 2021

from code review looks good,
but unfortunately cannot run local unittest, do not have php 5 installed, and it somehow not work with 8

@RickR2H
Copy link
Member

RickR2H commented Aug 2, 2021

I have tested this item ✅ successfully on d02dfd8

Patch works.

For people testing on Windows, make sure PHP is added to your system path variables.
Then use: php ./libraries/vendor/phpunit/phpunit/phpunit ./tests/unit/suites/libraries/joomla/language/JLanguageTest.php


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35026.

@RickR2H
Copy link
Member

RickR2H commented Aug 3, 2021

@ricardo1709 please test 😉


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35026.

@ricardo1709
Copy link
Contributor

I have tested this item ✅ successfully on d02dfd8

The patch works.
I have tested it on WSL


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35026.

@richard67
Copy link
Member Author

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/35026.

@joomla-cms-bot joomla-cms-bot added PR-staging Unit/System Tests RTC This Pull Request is Ready To Commit labels Aug 4, 2021
@wilsonge wilsonge merged commit ca04b45 into joomla:staging Aug 4, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 4, 2021
@wilsonge
Copy link
Contributor

wilsonge commented Aug 4, 2021

Merging as this just affects tests

@wilsonge wilsonge modified the milestones: Joomla 3.10.0, Joomla! 3.9.29 Aug 4, 2021
@richard67 richard67 deleted the staging-fix-unit-tests-for-language-plural-suffixes branch August 5, 2021 06:05
@zero-24 zero-24 modified the milestones: Joomla! 3.9.29, Joomla 3.10.0 Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants