Skip to content
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

Bug 1132711 - Convert Mozmill testSecurityNotification #95

Closed
wants to merge 3 commits into from

Conversation

@galgeek
Copy link
Collaborator

@galgeek galgeek commented Feb 20, 2015

def setUp(self):
FirefoxTestCase.setUp(self)

self.test_data = [

This comment has been minimized.

@whimboo

whimboo Feb 20, 2015
Contributor

To be consistent with other tests please name this self.urls.

'http://www.mozqa.com'
]

self.identity_box = self.browser.navbar.locationbar.identity_popup.box

This comment has been minimized.

@whimboo

whimboo Feb 20, 2015
Contributor

Please cache a Marionette object but not an underlying DOM node.

This comment has been minimized.

@whimboo

whimboo Feb 20, 2015
Contributor

Sorry, my failure. This is fine, given that it's a Marionette HTMLElement instance.


def tearDown(self):
try:
self.windows.close_all([self.browser])

This comment has been minimized.

@whimboo

whimboo Feb 20, 2015
Contributor

Also this test is not opening new windows. So this code is not necessary.

FirefoxTestCase.tearDown(self)

def test_security_notification(self):

This comment has been minimized.

@whimboo

whimboo Feb 20, 2015
Contributor

nit: please kill those first empty lines in functions across your patches.

# Verify the text in Technical Content contains the page with invalid cert
text = (self.marionette.find_element(By.ID, 'technicalContentText')).get_attribute(
'textContent')
self.assertTrue(self.test_data[0][8:] in text)

This comment has been minimized.

@whimboo

whimboo Feb 20, 2015
Contributor

Please separate the retrieval of elements and their usage. You can also use assertIn here.

self.assertIsNotNone(self.marionette.find_element(By.ID, 'exceptionDialogButton'))

# Verify the error code is correct
self.assertTrue('sec_error_expired_certificate' in text)

This comment has been minimized.

@whimboo

whimboo Feb 20, 2015
Contributor

Same as above. You can use assertIn here.

@galgeek galgeek self-assigned this Feb 20, 2015
self.wait_for_condition(lambda _: self.identity_box.get_attribute('className') ==
'unknownIdentity')

self.marionette.set_context('content')

This comment has been minimized.

@whimboo

whimboo Feb 23, 2015
Contributor

To be safe you should use with self.marionette.set_context('content') here. Otherwise we stay in content for teardown if anything below fails.

# Go to a site that has an invalid (expired) cert
self.assertRaises(MarionetteException, self.marionette.navigate, self.urls[0])

# Wait for about:error page

This comment has been minimized.

@whimboo

whimboo Feb 23, 2015
Contributor

Please update this comment regarding DOM and being able to receive events as done for the other PRs.

@galgeek galgeek force-pushed the galgeek:securityNotification branch from 5745761 to ec0092f Feb 23, 2015
@galgeek galgeek force-pushed the galgeek:securityNotification branch from ec0092f to f64b6a2 Feb 23, 2015
@whimboo
Copy link
Contributor

@whimboo whimboo commented Feb 24, 2015

PR has been merged as 3f119d8

@whimboo whimboo closed this Feb 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants