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

feat: Language translations finder and update #7896

Merged
merged 46 commits into from
Sep 18, 2023

Conversation

neznaika0
Copy link
Contributor

@neznaika0 neznaika0 commented Sep 4, 2023

Description
Supersedes #7889

See thread https://forum.codeigniter.com/showthread.php?tid=88299
Ready to discuss the solution.
Edit #7889: Start with branch 4.5

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added 4.5 new feature PRs for new features labels Sep 4, 2023
@kenjis
Copy link
Member

kenjis commented Sep 5, 2023

Tests failed.

https://github.com/codeigniter4/CodeIgniter4/actions/runs/6077247490/job/16498118276?pr=7896

There were 3 failures:

1) CodeIgniter\Commands\Translation\LocalizationFinderTest::testUpdateDefaultLocale
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
     'DESCRIPTION' => 'TranslationOne.DESCRIPTION'
     'subTitle' => 'TranslationOne.subTitle'
     'overflow_style' => 'TranslationOne.overflow_style'
-    'last_operation_success' => 'TranslationOne.last_operation_success'
     'metaTags' => 'TranslationOne.metaTags'
     'Copyright' => 'TranslationOne.Copyright'
+    'last_operation_success' => 'TranslationOne.last_operation_success'
 )

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Commands/Translation/LocalizationFinderTest.php:202
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Commands/Translation/LocalizationFinderTest.php:46
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

2) CodeIgniter\Commands\Translation\LocalizationFinderTest::testUpdateWithLocaleOption
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
     'DESCRIPTION' => 'TranslationOne.DESCRIPTION'
     'subTitle' => 'TranslationOne.subTitle'
     'overflow_style' => 'TranslationOne.overflow_style'
-    'last_operation_success' => 'TranslationOne.last_operation_success'
     'metaTags' => 'TranslationOne.metaTags'
     'Copyright' => 'TranslationOne.Copyright'
+    'last_operation_success' => 'TranslationOne.last_operation_success'
 )

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Commands/Translation/LocalizationFinderTest.php:202
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Commands/Translation/LocalizationFinderTest.php:58
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

3) CodeIgniter\Commands\Translation\LocalizationFinderTest::testUpdateWithEmptyDirOption
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
     'DESCRIPTION' => 'TranslationOne.DESCRIPTION'
     'subTitle' => 'TranslationOne.subTitle'
     'overflow_style' => 'TranslationOne.overflow_style'
-    'last_operation_success' => 'TranslationOne.last_operation_success'
     'metaTags' => 'TranslationOne.metaTags'
     'Copyright' => 'TranslationOne.Copyright'
+    'last_operation_success' => 'TranslationOne.last_operation_success'
 )

/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Commands/Translation/LocalizationFinderTest.php:202
/home/runner/work/CodeIgniter4/CodeIgniter4/tests/system/Commands/Translation/LocalizationFinderTest.php:78
phpvfscomposer:///home/runner/work/CodeIgniter4/CodeIgniter4/vendor/phpunit/phpunit/phpunit:106

@neznaika0
Copy link
Contributor Author

I see. But my test is running.

aleksandr@aleksandr-debian:~/www/codeigniter$ vendor/bin/phpunit --color=always --no-coverage ./tests/system/Commands/Translation/LocalizationFinderTest.php
PHPUnit 9.6.10 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.7
Configuration: /home/aleksandr/www/codeigniter/phpunit.xml.dist

......                                                                                                                                                           6 / 6 (100%)

Nexus\PHPUnit\Extension\Tachycardia identified these 3 slow tests:
⚠  Took 1.17s from 0.50s limit to run CodeIgniter\\Commands\\Translation\\LocalizationFinderTest::testUpdateDefaultLocale
⚠  Took 1.16s from 0.50s limit to run CodeIgniter\\Commands\\Translation\\LocalizationFinderTest::testUpdateWithLocaleOption
⚠  Took 1.16s from 0.50s limit to run CodeIgniter\\Commands\\Translation\\LocalizationFinderTest::testUpdateWithEmptyDirOption


Time: 00:03.484, Memory: 12.00 MB

OK (6 tests, 18 assertions)

Why isn't there a key when there is one?

@kenjis
Copy link
Member

kenjis commented Sep 5, 2023

Probably you need to sort the order of the found files.

@neznaika0 neznaika0 force-pushed the feat-4.5-lang-finder branch from 2df8999 to c776a05 Compare September 5, 2023 12:11
Copy link
Contributor Author

@neznaika0 neznaika0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to correct the inaccuracies. And the tests after sorting work.

@kenjis
Copy link
Member

kenjis commented Sep 8, 2023

Please add an entry for this feature in the changelog Enhancements > Commands.
See https://github.com/codeigniter4/CodeIgniter4/blob/4.5/user_guide_src/source/changelogs/v4.5.0.rst#commands

Copy link
Member

@kenjis kenjis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kenjis kenjis merged commit 8a8865b into codeigniter4:4.5 Sep 18, 2023
@kenjis
Copy link
Member

kenjis commented Sep 18, 2023

@neznaika0 Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature PRs for new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants