Skip to content

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

Merged
seanbudd merged 6 commits into
nvaccess:masterfrom
CyrilleB79:sysTestEnglish
Sep 26, 2022
Merged

Make system tests pass when run locally on non-English systems#14148
seanbudd merged 6 commits into
nvaccess:masterfrom
CyrilleB79:sysTestEnglish

Conversation

@CyrilleB79

@CyrilleB79 CyrilleB79 commented Sep 14, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

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:

  • 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 (tested by @lukaszgo1 on Windows 10)
  • System tests pass on appVeyor

Known issues with pull request:

I have not the opportunity to test it myself.
But according to @lukaszgo1's tests, Notepad system test fail on Windows 11.
Since it is unrelated to the changes of this PR, I suggest to handle in a different issue/PR if needed.

Change log entries:

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

Code Review Checklist:

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

Cc @lukaszgo1, @MarcoZehe

@CyrilleB79
Make system tests pass when run locally on non-English systems.
31fd31f 
For this, the following changes have been made:
- NVDA interface language forced to English (in config files)
- Chrome UI language forced to English (via command line parameter)
- HTML sample content forced to English (via lang HTML attribute)
- Removed the unneeded NVDA+\ custom gesture which cannot be executed without modifier on some keyboard layout (e.g. French)

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.
@AppVeyorBot

AppVeyorBot commented Sep 14, 2022

Copy link
Copy Markdown

See test results for failed build of commit 6435cdd4c9

@CyrilleB79

CyrilleB79 commented Sep 14, 2022

Copy link
Copy Markdown
Contributor Author

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

@CyrilleB79 CyrilleB79 marked this pull request as ready for review September 14, 2022 15:43
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner September 14, 2022 15:43
@CyrilleB79 CyrilleB79 requested a review from seanbudd September 14, 2022 15:43
@lukaszgo1

lukaszgo1 commented Sep 14, 2022

Copy link
Copy Markdown
Contributor

@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.

seanbudd
seanbudd reviewed Sep 15, 2022
View reviewed changes

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally LGTM, one minor suggestion

Comment thread tests/system/robot/chromeTests.robot
@CyrilleB79

CyrilleB79 commented Sep 15, 2022

Copy link
Copy Markdown
Contributor Author

@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 CyrilleB79 marked this pull request as draft September 15, 2022 07:25
@CyrilleB79

CyrilleB79 commented Sep 22, 2022

Copy link
Copy Markdown
Contributor Author

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!

@lukaszgo1

lukaszgo1 commented Sep 22, 2022

Copy link
Copy Markdown
Contributor

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.

@CyrilleB79

CyrilleB79 commented Sep 22, 2022

Copy link
Copy Markdown
Contributor Author

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 CyrilleB79 requested a review from seanbudd September 22, 2022 20:42
@CyrilleB79 CyrilleB79 marked this pull request as ready for review September 22, 2022 20:42
@seanbudd

seanbudd commented Sep 26, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

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 seanbudd merged commit 7948008 into nvaccess:master Sep 26, 2022
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2023.1

Development

Successfully merging this pull request may close these issues.

When running tests locally, NVDA depends on the system language to be English

5 participants

@CyrilleB79 @AppVeyorBot @lukaszgo1 @seanbudd @nvaccessAuto