-
-
Notifications
You must be signed in to change notification settings - Fork 626
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
System tests with HTML samples. #10553
Conversation
The appveyor docs tell me that Internet Explorer 11, two versions of Chrome, and two versions for Firefox are installed. |
This comment has been minimized.
This comment has been minimized.
Adds one basic chrome test. Explicitly set Python version when running NVDA for system tests as test runner does not inherit environment variables.
ef1ca80
to
736e2c0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Ignore separator in application title. - Allow variations in the number of tab presses required to get to the start marker.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this pretty hard to review, at it is not clear to me what's new code and what has been moved somehow. How would you advise to review this?
Yeah, I understand. The main thing is getting feedback to know if it's easy to understand how the tests are written. Would you be able to write a system test after looking at this for a bit? Edit: I have also updated the description of this PR to point out the conventions used:
|
Since mostly the code has only been re-organized, the most important thing is to ensure that the tests are easy to understand these are the files in Other than that, one interesting change is how to interact with the remote library |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the changes, including being able to read the tests themselves. I would like however to see a second chrome test added before this is merged, so we have the confidence that multiple chrome tests don't cause strangeness with Chrome trying to be started twice or what ever would happen.
As we discussed, I have run the same test multiple times (leaving chrome open) which did not cause any issues. I'll go ahead and merge this. |
Example from "Fix double speaking label in angular checkbox" #10552
Summary of the issue:
Description of how this pull request fixes the issue:
/libraries
namedSomethingLib
the name of the module must match the name of the class used as the library, so the module is given an initial capital letter. Private functions / variables should all start with underscore. Public library "keywords" should use underscore to separate words.run_keyword
no longer has to be called manually.Overview of the refactoring done:
NVDA Core/startupShutdownNVDA.robot
robot/startupShutdownNVDA.py
robot/startupShutdownNVDA.robot
libraries/nvdaRobotLib.py
libraries/NvdaLib.py
libraries/SystemTestSpy/configManager.py
libraries/systemTestUtils.py
becomeslibraries/SystemTestSpy/blockUntilConditionMet.py
.libraries/helperLib.py
code fromlibraries/AssertsLib.py
libraries/sendKey.py
becomeslibraries/KeyInputLib.py
basically a straight rename.libraries/speechSpy.py
becomeslibraries/SystemTestSpy/speechSpySynthDriver.py
libraries/systemTestSpy.py
becomeslibraries/SystemTestSpy/speechSpyGlobalPlugin.py
NVDA Core/variables.py
no longer necessary, strings can easily be defined in test code now written in PythonThe chrome tests are added:
robot/chromeTests.py
robot/chromeTests.robot
Testing performed:
Ran the tests locally and on Appveyor
Known issues with pull request:
Doesn't close chrome at the completion of the test.
Change log entry:
Section: Changes for developers:
Automated testing of NVDA with Chrome and a HTML sample is now possible.