Permaorange in webdriver/navigation.py when Gecko 55 merges to beta on 2017-06-12

RESOLVED WORKSFORME

Status

defect
RESOLVED WORKSFORME
2 years ago
7 months ago

People

(Reporter: philor, Unassigned)

Tracking

(Depends on 2 bugs)

55 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 unaffected, firefox55blocking disabled, firefox66 unaffected)

Details

Attachments

(2 attachments)

Three permaorange unexpected passes (which have been around since the mid-May wpt update, but I didn't notice them below all of the bug 1358020 failures):

https://treeherder.mozilla.org/logviewer.html#?job_id=102582424&repo=try

TEST-UNEXPECTED-PASS | /webdriver/navigation.py | navigation.py::test_get_current_url_alert_prompt - expected FAIL
INFO - TEST-UNEXPECTED-PASS | /webdriver/navigation.py | navigation.py::test_set_malformed_url - expected FAIL
INFO - TEST-UNEXPECTED-OK | /webdriver/navigation.py | expected TIMEOUT

which is nice as a hint for the apparently-unfiled bug about expecting failure, the reason those two fail is because of something that's only enabled on nightly, but shorter range, it's only two weeks until we merge to beta and

[Tracking Requested - why for this release]: merge bustage, closed tree, delayed b1, wailing and gnashing of teeth.
marking as blocker for 55.  James, is this something you can look at?
Flags: needinfo?(james)
Posted patch 1368262.diffSplinter Review
Something like that?
Flags: needinfo?(james)
James, did you mean to request review on that patch?
Flags: needinfo?(james)
Sure I guess? Not sure who should review it. Previously with this kind of thing someone who knows how has done a beta simulation run with the patch applied and it has landed as a=testonly or similar. I don't know how to request that more than needinfoing a random sheriff though.
Flags: needinfo?(james)
Roger that, let's ni? phil. :)
Flags: needinfo?(philringnalda)
Where's the dependency on a bug about adding awareness of RELEASE_OR_BETA to wpt .inis so we don't have to land things on a closed tree after the merge to unbust stuff?

Where's the dependency on a bug about investigating what thing which is enabled on trunk and disabled on RELEASE_OR_BETA causes the test to fail on trunk and pass on beta?
James, can you please reply to comment 6? FYI, this is currently marked as blocking Monday's merge of 55 to Beta.
Flags: needinfo?(james)
> Where's the dependency on a bug about adding awareness of RELEASE_OR_BETA to wpt .inis so we don't have to land things on a closed tree after the merge to unbust stuff?

That's in the mozinfo.json files now? Then we can use it. In fact it probably already works, although we will never add it as metadata when updating. Anyway filed.

Why aren't we keeping a tree that's m+c + patches to run it as beta + patches required for beta specifically, and rebasing it on m-c every time we want to do a new simulation run (or just with a cron job that does that once a day). Having to apply patches after the merge seems entirely avoidable.

> Where's the dependency on a bug about investigating what thing which is enabled on trunk and disabled on RELEASE_OR_BETA causes the test to fail on trunk and pass on beta?

I think it makes more sense to ask the test author/feature owner who happens to be ato in this case.
Blocks: 1371436
Flags: needinfo?(james) → needinfo?(ato)
No longer blocks: 1371436
Depends on: 1371436
I know gps has a "correct way to use DVCS, not this horrible awful CVS-inspired hack of separate repos" plan for how branching should work, that's the only thing different than the current scheme that we'll be allowed to change to, but actually listening to the details of ocean-boiling pie-in-the-sky pipe dreams just makes me sad, so I don't know what it is or whether it will make pre-landing things on a beta (or an infinite series of future betas) that doesn't yet exist possible.
Inconveniently, this patch won't work, since whatever unknown thing is failing on the trunk is also failing on DevEdition, which is currently green with the expectation that we'll fail, and with this patch would become permaorange. So the patch we need is the one which simply completely disables the test from running at all until someone wants to look at it, landed on the trunk, before Monday morning.
Flags: needinfo?(philringnalda)
(In reply to Phil Ringnalda (:philor) from comment #6)
> Where's the dependency on a bug about adding awareness of RELEASE_OR_BETA to
> wpt .inis so we don't have to land things on a closed tree after the merge
> to unbust stuff?
> 
> Where's the dependency on a bug about investigating what thing which is
> enabled on trunk and disabled on RELEASE_OR_BETA causes the test to fail on
> trunk and pass on beta?

I don’t understand exactly what you’re asking for here, but as far as I can
tell from jgraham’s attached patch, two tests are getting re-enabled because
they have started passing?

> -[navigation.py]
> -  type: wdspec
> -  expected: TIMEOUT
> -  [navigation.py::test_get_current_url_alert_prompt]
> -    expected: FAIL
> -
> -  [navigation.py::test_set_malformed_url]
> -    expected: FAIL

I believe the first test, test_get_current_url_alert_prompt, was fixed by
https://bugzilla.mozilla.org/show_bug.cgi?id=1359004, which correctly is only
available on 55.

The second I’m a bit more unsure about because many changes have been made in
this area, but I think it can be tracked down to
https://bugzilla.mozilla.org/show_bug.cgi?id=1335778.  Curiously, this is
already available on 54 because it was uplifted.
Flags: needinfo?(ato)
I built m-c with official branding locally and it passes testing/web-platform/tests/webdriver/navigation.py.
jgraham and I investigated this some more.  We found that when the RELEASE_OR_BETA ifdef is set, which can be caused by removing the “a” (alpha) character from config/milestone.txt, this causes sandboxed processes to get disabled.  This causes testing/web-platform/tests/webdriver/navigation.py’s file: URL test to start passing (which is an unexpected pass) because it is no longer reloading the content frame script.

With sandboxed processes enabled, content frame scripts are reloaded as Marionette navigates to a file: URL, but Marionette currently assumes that a frame script reload is always linked to when a remoteness change occurs.  There is https://bugzil.la/1332122 to fix this assumption.
Depends on: 1371709
Comment on attachment 8876158 [details]
Bug 1368262 - Disable webdriver test that behaves differently on dev edition vs release,

https://reviewboard.mozilla.org/r/147566/#review152152
Attachment #8876158 - Flags: review?(ato) → review+
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/1a3550df4043
Disable webdriver test that behaves differently on dev edition vs release, r=ato
https://hg.mozilla.org/mozilla-central/rev/1a3550df4043
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
We should keep the bug open for the remaining work to get the test re-enabled. Just marking it disabled for 55.0.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla55 → ---

At some point in the past this was fixed. There is no disable line anymore for any of the navigation tests.

Status: REOPENED → RESOLVED
Closed: 2 years ago7 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.