fix: validate_translation works even with unicode errors

 - added tests to ensure the file is covered
 - redirected the stderr output to be printed in the last step for easy
   debugging
 - cleanup unused imports in other test files

.github/workflows/python-tests.yml

@@ -14,6 +14,10 @@ jobs:
       - name: clone openedx/openedx-translations
         uses: actions/checkout@v3
 
+      - name: Install gettext
+        run: |
+          sudo apt install -y gettext
+
       # Sets up Python
       - name: setup python
         uses: actions/setup-python@v4

scripts/tests/mock_translations_dir/demo-xblock/conf/locale/ar/LC_MESSAGES/django.po

@@ -0,0 +1,28 @@
+# #-#-#-#-#  django.po (course-discovery)  #-#-#-#-#
+# edX translation file
+# Copyright (C) 2018 edX
+# This file is distributed under the GNU AFFERO GENERAL PUBLIC LICENSE.
+#
+# Translators:
+# Translators:
+# Omar Al-Ithawi <[email protected]>, 2023
+# Brian Smith, 2023
+#
+msgid ""
+msgstr ""
+"Project-Id-Version: edx-platform\n"
+"Report-Msgid-Bugs-To: \n"
+"POT-Creation-Date: 2023-10-03 00:20+0000\n"
+"PO-Revision-Date: 2023-01-26 15:39+0000\n"
+"Last-Translator: Brian Smith, 2023\n"
+"Language-Team: Arabic (https://app.transifex.com/open-edx/teams/147691/ar/)\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=UTF-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Language: ar\n"
+"Plural-Forms: nplurals=6; plural=n==0 ? 0 : n==1 ? 1 : n==2 ? 2 : n%100>=3 && n%100<=10 ? 3 : n%100>=11 && n%100<=99 ? 4 : 5;\n"
+
+#: apps/api/filters.py:47
+#, python-brace-format
+msgid "No user with the username [{username}] exists."
+msgstr "لا يوجد مستخدم بهذا الاسم [{username}]."

scripts/tests/mock_translations_dir/demo-xblock/conf/locale/de_DE/LC_MESSAGES/django.po

@@ -0,0 +1,19 @@
+msgid ""
+msgstr ""
+"Project-Id-Version: 0.1a\n"
+"Report-Msgid-Bugs-To: [email protected]\n"
+"POT-Creation-Date: 2023-10-15 20:36+0000\n"
+"PO-Revision-Date: 2023-10-15 20:36:20.949198\n"
+"Last-Translator: \n"
+"Language-Team: openedx-translation <[email protected]>\n"
+"Language: de_DE\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=utf-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"Generated-By: Babel 2.8.0\n"
+
+#: demo_file.py:5
+#, python-brace-format
+msgid "{action} is not a valid action."
+msgstr "{action} ist keine gültige Aktion."

scripts/tests/mock_translations_dir/demo-xblock/conf/locale/en/LC_MESSAGES/django.po

@@ -0,0 +1,6 @@
+msgid ""
+msgstr ""
+
+#, python-brace-format
+msgid "{action} is not a valid action."
+msgstr ""

scripts/tests/mock_translations_dir/demo-xblock/conf/locale/hi/LC_MESSAGES/django.po

@@ -0,0 +1,18 @@
+msgid ""
+msgstr ""
+"Project-Id-Version: 0.1a\n"
+"Report-Msgid-Bugs-To: [email protected]\n"
+"POT-Creation-Date: 2023-10-15 20:36+0000\n"
+"PO-Revision-Date: 2023-10-15 20:36:20.949198\n"
+"Last-Translator: \n"
+"Language-Team: openedx-translation <[email protected]>\n"
+"Language: de_DE\n"
+"MIME-Version: 1.0\n"
+"Content-Type: text/plain; charset=utf-8\n"
+"Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=2; plural=(n != 1);\n"
+"Generated-By: Babel 2.8.0\n"
+
+#, python-brace-format
+msgid "{action} is not a valid action."
+msgstr "{कार्रवाई} एक वैध कार्रवाई नहीं है।"  # Invalid brace-format with encoding errors

scripts/tests/test_fix_transifex_resource_names.py

@@ -1,7 +1,7 @@
 """
 Tests for fix_transifex_resource_names.py.
 """
-import unittest
+
 from unittest.mock import MagicMock
 from ..fix_transifex_resource_names import get_repo_slug_from_resource
 

scripts/tests/test_sync_translations.py

@@ -3,7 +3,6 @@
 """
 import types
 
-import pytest
 import responses
 from transifex.api import transifex_api, Project
 from transifex.api.jsonapi import Resource

scripts/tests/test_validate_translation_files.py

@@ -0,0 +1,67 @@
+"""
+Tests for the validate_translation_files.py script.
+"""
+
+import os.path
+
+from ..validate_translation_files import (
+    get_translation_files,
+    main,
+)
+
+SCRIPT_DIR = os.path.dirname(__file__)
+
+
+def test_get_translation_files():
+    """
+    Ensure `get_translation_files` skips the source translation files and non-po files.
+    """
+    mock_translations_dir = os.path.join(SCRIPT_DIR, 'mock_translations_dir')
+    po_files_sorted = sorted(get_translation_files(mock_translations_dir))
+    relative_po_files = [
+        os.path.relpath(po_file, SCRIPT_DIR)
+        for po_file in po_files_sorted
+    ]
+
+    assert relative_po_files == [
+        'mock_translations_dir/demo-xblock/conf/locale/ar/LC_MESSAGES/django.po',
+        'mock_translations_dir/demo-xblock/conf/locale/de_DE/LC_MESSAGES/django.po',
+        'mock_translations_dir/demo-xblock/conf/locale/hi/LC_MESSAGES/django.po',
+    ]
+
+
+def test_main_on_invalid_files(capsys):
+    """
+    Integration test for the `main` function on some invalid files.
+    """
+    mock_translations_dir = os.path.join(SCRIPT_DIR, 'mock_translations_dir')
+    exit_code = main(mock_translations_dir)
+    out, err = capsys.readouterr()
+
+    assert 'VALID:' in out, 'Valid files should be printed in stdout'
+    assert 'de_DE/LC_MESSAGES/django.po' in out, 'German translation file should be found valid'
+    assert 'ar/LC_MESSAGES/django.po' in out, 'Arabic translation file should be found valid'
+    assert 'hi/LC_MESSAGES/django.po' not in out, 'Invalid file should be printed in stderr'
+    assert 'en/LC_MESSAGES/django.po' not in out, 'Source file should not be validated'
+
+    assert 'INVALID:' in err
+    assert 'hi/LC_MESSAGES/django.po' in err
+    assert '\'msgstr\' is not a valid Python brace format string, unlike \'msgid\'' in err
+    assert 'FAILURE: Some translations are invalid.' in err
+
+    assert exit_code == 1, 'Should fail due to invalid hi/LC_MESSAGES/django.po file'
+
+
+def test_main_on_valid_files(capsys):
+    """
+    Integration test for the `main` function but only for the Arabic translations which is valid.
+    """
+    mock_translations_dir = os.path.join(SCRIPT_DIR, 'mock_translations_dir/demo-xblock/conf/locale/ar')
+    exit_code = main(mock_translations_dir)
+    out, err = capsys.readouterr()
+
+    assert 'VALID:' in out, 'Valid files should be printed in stdout'
+    assert 'ar/LC_MESSAGES/django.po' in out, 'Arabic translation file is valid'
+    assert 'SUCCESS: All translation files are valid.' in out
+    assert not err.strip(), 'The stderr should be empty'
+    assert exit_code == 0, 'Should succeed due in validating the ar/LC_MESSAGES/django.po file'

scripts/validate_translation_files.py

@@ -28,43 +28,58 @@ def validate_translation_file(po_file):
         ['msgfmt', '-v', '--strict', '--check', po_file],
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
-        text=True,
     )
+
+    stdout = completed_process.stdout.decode(encoding='utf-8', errors='replace')
+    stderr = completed_process.stderr.decode(encoding='utf-8', errors='replace')
+
     return {
         'valid': completed_process.returncode == 0,
-        'output': completed_process.stdout + '\n' + completed_process.stderr
+        'output': f'{stdout}\n{stderr}',
     }
 
 
-def main():
+def main(translations_dir='translations'):
     """
-    Run msgfmt and print errors to stderr.
+    Run GNU gettext `msgfmt` and print errors to stderr.
+
+    Returns integer OS Exit code:
+
+      return 0 for valid translation.
+      return 1 for invalid translations.
     """
     translations_valid = True
 
-    po_files = get_translation_files('translations')
+    invalid_lines = []
+
+    po_files = get_translation_files(translations_dir)
     for po_file in po_files:
         result = validate_translation_file(po_file)
 
         if result['valid']:
             print('VALID: ' + po_file)
             print(result['output'], '\n' * 2)
         else:
-            print('INVALID: ' + po_file, file=sys.stderr)
-            print(result['output'], '\n' * 2, file=sys.stderr)
+            invalid_lines.append('INVALID: ' + po_file)
+            invalid_lines.append(result['output'] + '\n' * 2)
             translations_valid = False
 
-    print('-----------------------------------------')
+    # Print validation errors in the bottom for easy reading
+    print('\n'.join(invalid_lines), file=sys.stderr)
+
     if translations_valid:
+        print('-----------------------------------------')
         print('SUCCESS: All translation files are valid.')
+        print('-----------------------------------------')
         exit_code = 0
     else:
-        print('FAILURE: Some translations are invalid. Check the stderr for error messages.')
+        print('---------------------------------------', file=sys.stderr)
+        print('FAILURE: Some translations are invalid.', file=sys.stderr)
+        print('---------------------------------------', file=sys.stderr)
         exit_code = 1
-    print('-----------------------------------------')
 
-    sys.exit(exit_code)
+    return exit_code
 
 
 if __name__ == '__main__':
-    main()
+    sys.exit(main())  # pragma: no cover