testSecurityNotification.js fails due to timeout on cert_domain_link

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
P2
normal
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: ashughes, Assigned: AndreeaMatei)

Tracking

unspecified

Firefox Tracking Flags

(firefox16 fixed, firefox17 fixed, firefox18 fixed, firefox19 fixed, firefox-esr10 fixed)

Details

(Whiteboard: [mozmill-test-failure] s=121015 u=failure c=security p=1, URL)

Attachments

(3 attachments, 2 obsolete attachments)

The following test failure was noticed today during the 9.0b5 test runs across all platforms:

Filename: /testSecurity/testSecurityNotification.js
Test: testSecurityNotification.js :: testSecNotification
Error: Timeout exceeded for waitForElementID: cert_domain_link

Updated

6 years ago
Assignee: nobody → remus.pop
Created attachment 580044 [details] [diff] [review]
skip test (all branches)

Skips the test.
Attachment #580044 - Flags: review?(vlad.mozbugs)
Comment on attachment 580044 [details] [diff] [review]
skip test (all branches)

Same as for the other skip patch - remember to come back on this one. Thanks! :)
Attachment #580044 - Flags: review?(vlad.mozbugs) → review+
This is related to bug 708494 because of the webpage we use to test security elements.
See Also: → bug 708494
Comment on attachment 580044 [details] [diff] [review]
skip test (all branches)

Please remember to hand off reviews to me for check-in. Also, your commit message is worded backwards. I will have to upload a fixed patch.
Attachment #580044 - Flags: review+
Created attachment 580085 [details] [diff] [review]
disable test (all) [checked-in]
Attachment #580044 - Attachment is obsolete: true
Attachment #580085 - Flags: review+
Comment on attachment 580085 [details] [diff] [review]
disable test (all) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/eacfdfac2cad (default)
http://hg.mozilla.org/qa/mozmill-tests/rev/9bc4bc1a4926 (mozilla-aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/eef3503363a9 (mozilla-beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/6d52b1e6d76f (mozilla-release)

Patch needed to disable this test on mozilla-1.9.2
Attachment #580085 - Attachment description: disable test (all) → disable test (all) [checked-in]
Status: NEW → ASSIGNED
Whiteboard: [mozmill-test-failure] → [mozmill-test-failure][mozmill-test-skipped]
Created attachment 580090 [details] [diff] [review]
disable test (1.9.2) [checked-in]
Attachment #580090 - Flags: review?(hskupin)
Comment on attachment 580090 [details] [diff] [review]
disable test (1.9.2) [checked-in]

Usually we should only add the skipping code here. Otherwise it could cause issues with the merge process if skipping a test only applies to some but not all branches. Please change that. Otherwise r=me.
Attachment #580090 - Flags: review?(hskupin) → review+
(In reply to Henrik Skupin (:whimboo) from comment #8)
> Comment on attachment 580090 [details] [diff] [review]
> disable test (1.9.2)
> 
> Usually we should only add the skipping code here. Otherwise it could cause
> issues with the merge process if skipping a test only applies to some but
> not all branches. Please change that. Otherwise r=me.

We've been in the habit of removing the deprecated "Litmus" lines for some time now. The all-branches patch above includes it. Also, we will never merge 1.9.2 into one of the rapid release channels, will we?

Given that, I think this patch is fine to check in as is; do you disagree?
Given that we possibly will not backout this patch go ahead. I just wanted to add this information for the future, so it's clear when we have such a case.
Comment on attachment 580090 [details] [diff] [review]
disable test (1.9.2) [checked-in]

Landed:
http://hg.mozilla.org/qa/mozmill-tests/rev/3f0b9f114a84 (mozilla-1.9.2)
Attachment #580090 - Attachment description: disable test (1.9.2) → disable test (1.9.2) [checked-in]
I think the problem with this test is that https://mozilla.org and http://www.mozilla.org are no longer valid test sites (the certs have been fixed).

Comment 13

5 years ago
I am working on this bug to fix

Comment 14

5 years ago
I am working on this bug to fix
Assignee: remus.pop → sidharthchugh
Thanks Sidharth. Please let me know via email (ahughes@mozilla.com) or IRC (ashughes) if you need help getting started.
Sidharth, we haven't heard from you for a longer time. Are you still ok with working on a fix for this failure?
Given that Sidharth sadly didn't get back to us we have to move forward on this test failure and get the test re-enabled. Putting it on our list for the next week.
Assignee: sidharthchugh → nobody
Status: ASSIGNED → NEW
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] → [mozmill-test-failure][mozmill-test-skipped] s=2012-8-27 u=failure c=security p=1
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] s=2012-8-27 u=failure c=security p=1 → [mozmill-test-failure][mozmill-test-skipped] s=q3 u=failure c=security p=1
(Assignee)

Updated

5 years ago
Assignee: nobody → andreea.matei
Status: NEW → ASSIGNED
(Assignee)

Comment 18

5 years ago
It looks like we fail because the last loaded page (https://mozilla.org/) should be one with an invalid certificate so the buttons "Get me out of here" and "Add Exception" be present. 
Instead, that page is a secure one, with Green Larry.

I have tested with an unsecure page, but it still fails at finding the "cert_domain_link" ID. 
It appears that we only have the technicalContentText which contains the whole text from Technical Details. 
So I propose we use that ID and an expect.contain() to check the link.

Another thing I've noticed in this test is that the first loaded page (https://addons.mozilla.org/licenses/5.txt) is a "Not Found" page, but it is a secure one so there's no failure there.
(Assignee)

Comment 19

5 years ago
Created attachment 663385 [details] [diff] [review]
patch v1

Removed time constants that are unnecessary, added constants for the visited pages and as discussed in "Ask the experts" meeting and on IRC, used "https://summitbook.mozilla.org/" for the invalid cert page as well as expect.match() for checking the correct link.
Attachment #663385 - Flags: review?(hskupin)
Attachment #663385 - Flags: review?(dave.hunt)
Comment on attachment 663385 [details] [diff] [review]
patch v1

Review of attachment 663385 [details] [diff] [review]:
-----------------------------------------------------------------

::: tests/functional/testSecurity/testSecurityNotification.js
@@ +37,3 @@
>    controller.waitForPageLoad();
>  
>    // Verify the link in Technical Details is correct

This test is now using an expired certificate rather than a mismatching one, so there is no link in the page. See http://mxr.mozilla.org/mozilla-central/source/browser/components/certerror/content/aboutCertError.xhtml#130

@@ +38,5 @@
>  
>    // Verify the link in Technical Details is correct
> +  var text = new elementslib.ID(controller.tabs.activeTab, "technicalContentText");
> +  controller.waitForElement(text);
> +  expect.match(text.getNode().textContent, /([-\w\.]+)+?/,

This looks like a very loose match. If we want to check that the page URL appears in the text we should use INVALID_CERT_PAGE.
Attachment #663385 - Flags: review?(hskupin)
Attachment #663385 - Flags: review?(dave.hunt)
Attachment #663385 - Flags: review-
Priority: -- → P2
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] s=q3 u=failure c=security p=1 → [mozmill-test-failure][mozmill-test-skipped] s=121015 u=failure c=security p=1
(Assignee)

Comment 21

5 years ago
Created attachment 673833 [details] [diff] [review]
patch v2

Updated accordingly to the review, used the invalid certificate page and substring to get rid of "http://" when checking the Technical details.
Attachment #663385 - Attachment is obsolete: true
Attachment #673833 - Flags: review?(hskupin)
Attachment #673833 - Flags: review?(dave.hunt)
Comment on attachment 673833 [details] [diff] [review]
patch v2

Review of attachment 673833 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Please check that we can backport the patch.
Attachment #673833 - Flags: review?(hskupin)
Attachment #673833 - Flags: review?(dave.hunt)
Attachment #673833 - Flags: review+
http://hg.mozilla.org/qa/mozmill-tests/rev/a742c618fc98 (default)
status-firefox-esr10: --- → affected
status-firefox16: --- → affected
status-firefox17: --- → affected
status-firefox18: --- → affected
status-firefox19: --- → fixed
(Assignee)

Comment 24

5 years ago
I've checked and this patch applies cleanly to all other branches.
http://hg.mozilla.org/qa/mozmill-tests/rev/ddbbb1cf51d5 (aurora)
http://hg.mozilla.org/qa/mozmill-tests/rev/ed29839958fb (beta)
http://hg.mozilla.org/qa/mozmill-tests/rev/9c2386fe032c (release)
http://hg.mozilla.org/qa/mozmill-tests/rev/054004a8aba1 (esr10)
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox-esr10: affected → fixed
status-firefox16: affected → fixed
status-firefox17: affected → fixed
status-firefox18: affected → fixed
Resolution: --- → FIXED
Whiteboard: [mozmill-test-failure][mozmill-test-skipped] s=121015 u=failure c=security p=1 → [mozmill-test-failure] s=121015 u=failure c=security p=1
You need to log in before you can comment on or make changes to this bug.