nvda icon indicating copy to clipboard operation
nvda copied to clipboard

Make system tests pass when run locally on non-English systems

Open CyrilleB79 opened this issue 1 year ago • 5 comments

Link to issue number:

Closes #13362

Summary of the issue:

The majority of system tests fail when run on non-English systems. On my system (French), failures are due to the following reasons:

  • Default NVDA's UI language is user's default Windows language, thus the messages of NVDA are reported in this language.
  • If not specified, the language of an HTML page is set by the browser itself which may not be English (system language?)
  • According to the locale keyboard layout, someemulated keypress may fail. E.g. "NVDA+" cannot be emulated with French keyboard layout since \ (backslash) can only be accessed with modifiers (AltGr).

In #13362, it was also stated that Chrome's language may make some tests fail. That's not the case on my French system. But we may imagine that the keyboard shortcuts could vary from one language to another, e.g. "Alt+d" to focus the address field.

Description of user facing changes

System tests now pass on French system. It should pass on any system regardless of its language.

Description of development approach

  1. NVDA interface language is forced to English in the various config files.
  2. HTML sample content forced to English (via lang HTML attribute)
  3. Removed the unneeded NVDA+\ custom gesture which cannot be executed without modifier on some keyboard layout (e.g. French). Actually, there is no need for custom gesture since NVDA+d is now a native gesture of NVDA for the same script. Even if not used anymore for now, I have not removed the possibility to use a custom gesture file in case an unassigned script has to be tested in the future. Should it be the case, care should be taken to assign a gesture that can be executed on any keyboard layout, i.e. use a letter rather than punctuation as the main key name. E. Chrome UI language is forced to English via command line parameter (see List of Chromium Command Line Switches « Peter Beverloo)

Note: For the Chrome command line --lang parameter to be taken into account, it should be ensured that no previous Chrome window is open when the tests are run.

Testing strategy:

  • [x] System tests pass on my French local system Windows 10 20H2 (x64) build 19042.1889, Chrome 105 Only "NVDA" tag tested, not "installer".: runsystemtests.bat -i nvda
  • [ ] System test pass locally on an English system
  • [x] System tests pass on appVeyor

Known issues with pull request:

None

Change log entries:

Bug fixes (or For Developers ?) System test should now pass when run locally on non-English systems. (#13362)

Code Review Checklist:

  • [x] Pull Request description:
    • description is up to date
    • change log entries
  • [x] Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • [x] API is compatible with existing add-ons.
  • [x] Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • [x] UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • [x] Security precautions taken.

Cc @lukaszgo1, @MarcoZehe

CyrilleB79 avatar Sep 14 '22 14:09 CyrilleB79

  • PASS: Translation comments check.
  • PASS: Unit tests.
  • PASS: Lint check.
  • FAIL: System tests (tags: installer NVDA). See test results for more information.
  • Build (for testing PR): https://ci.appveyor.com/api/buildjobs/1n96fbxxdtlarutv/artifacts/output/nvda_snapshot_pr14148-26577,6435cdd4.exe
  • CI timing (mins): INIT 0.0, INSTALL_START 0.8, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 23.4, TESTSETUP_START 0.0, TESTSETUP_END 0.2, TEST_START 0.0, TEST_END 13.4, FINISH_END 0.2

See test results for failed build of commit 6435cdd4c9

AppVeyorBot avatar Sep 14 '22 14:09 AppVeyorBot

If someone would like to test with an English system, this would be helpful.

CyrilleB79 avatar Sep 14 '22 15:09 CyrilleB79

@CyrilleB79 Many thanks for taking this! Ive tested and on my English Windows 10 21H2 all system tests passes. Unfortunately on my Polish Windows 11 all notepad tests fail - it is hard to say if this is caused by this PR, since previously it was impossible to test on non-English systems at al. Would someone running English Windows 11, perhaps @josephsl be able to check if Notepad system tests from current master work for them? While this PR is certainly a pretty big improvement for contributors running non-English Windows and should be merged, it should also be noted that to enable testing NVDA in different languages (desirable as mentioned i.e. in #12710) a more sophisticated solution may need to be written in the future.

lukaszgo1 avatar Sep 14 '22 20:09 lukaszgo1

@lukaszgo1, could you have a look or provide the logs of one of the failing Notepad test to be able to investigate why it fails and if it can be fixed?

Converting to draft, waiting for a test on English Windows 11 (on this PR's branch and on last alpha).

CyrilleB79 avatar Sep 15 '22 07:09 CyrilleB79

Ping @lukaszgo1, @josephsl:

  • @josephsl would you be able to test this on Windows 11? If not (due to lack of time or any other reason), just let me know so that I ask to someone else, e.g. on nvda-devel
  • @lukaszgo1 would you mind provide information on the failing tests? Many thanks!

CyrilleB79 avatar Sep 22 '22 10:09 CyrilleB79

I haven't, yet, analyzed the logs from the failed tests, however I've switched my Windows 11 to English and can confirm that Notepad tests fail even on the current master. so the problem has not been introduced in this PR. Sorry for the confusion.

lukaszgo1 avatar Sep 22 '22 16:09 lukaszgo1

I haven't, yet, analyzed the logs from the failed tests,

Feel free to provide the log results if you want me to have a look.

however I've switched my Windows 11 to English and can confirm that Notepad tests fail even on the current master. so the problem has not been introduced in this PR.

Thanks for this valuable test and information.

Since the remaining issue is not directly linked to this PR nor to the corresponding issue (#13362), I set this PR ready for review.

CyrilleB79 avatar Sep 22 '22 20:09 CyrilleB79

Change log entries: Bug fixes (or For Developers ?)

I think the change log entry should be in "for developers" or not at all. I don't think changes that only affect developers should go in "bug fixes".

seanbudd avatar Sep 26 '22 01:09 seanbudd