-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Implement Send Alert Text tests for WPT wdspec tests #8155
Implement Send Alert Text tests for WPT wdspec tests #8155
Conversation
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.
Thanks for testing invalid input. The test is unnecessarily complicated and could be simplified using parametrization. I’ve included an example of how to make the test simpler.
Once you make that final change, I think this PR is ready for integration.
"number": 123} | ||
session.execute_script("window.result = window.prompt(\"Enter Your Name: \", \"Name\");") | ||
for item in elements: | ||
body = {"text": elements[item]} |
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.
This iterator is not very Pythonic. Can you change the test to use @pytest.parametrize
?
This would look roughly something like this:
@pytest.parametrize("text", [None, {}, [], 42, True])
def test_invalid_input(session, text):
session.execute_script("""window.result = window.prompt("foo")""")
response = send_alert_text(session, {"text": text})
assert_error(response, "invalid argument")
Note that I haven’t run the above code.
Also, there are conflicts which you must resolve. |
delete trailing whitespace Deleted unused import. Simplified send alert text test. Test non string input. Test text with whitespace. Formatting fixes. Add more tests for invalid input Initiate the prompt once only Parametrize invalid input test
Build PASSEDStarted: 2017-11-13 21:59:17 Failing Jobs
View more information about this build on: |
delete trailing whitespace
Deleted unused import. Simplified send alert text test.
Test non string input. Test text with whitespace. Formatting fixes.
Add more tests for invalid input