Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Description • 6 years ago

With the refactoring of the wdspec tests I haven't taken into account that some of the tests which require a restart of Firefox could still fail with webrender enabled. In current manifest files this is being tracked with:

>  disabled:
>    if webrender: bug 1425588

This bug will update all the failing tests and disables them for webrender for now.

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 1 • 6 years ago

I think that we should limit the patch to get_element_property and get_element_tag_name for now which have a higher failure rate. The others have only 3 failures per week, so I don't think we should disable those completely.

	Andreas Tolfsen ❲:ato❳
	Comment 3 • 6 years ago
mozreview-review

Comment on attachment 8975407 [details]
Bug 1461253 - [wdspec] Disable user prompt tests of get_element_property and get_element_tag_name for webrender.

https://reviewboard.mozilla.org/r/243706/#review249608

::: commit-message-87cc1:1
(Diff revision 1)
> +Bug 1461253 - [wdspec] Disable user_prompt tests for get_element_property and get_element_tag_name for webrender.

I think this commit message is way too long.

It also does not explain why they are being disabled for webrenderer
only?  I thought you wanted these tests to run so you could reproduce
the slow shutdown times?

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 4 • 6 years ago
mozreview-review-reply

Comment on attachment 8975407 [details]
Bug 1461253 - [wdspec] Disable user prompt tests of get_element_property and get_element_tag_name for webrender.

https://reviewboard.mozilla.org/r/243706/#review249608

> I think this commit message is way too long.
> 
> It also does not explain why they are being disabled for webrenderer
> only?  I thought you wanted these tests to run so you could reproduce
> the slow shutdown times?

> I think this commit message is way too long.

There is no restriction to the length and I see way longer commit messages in the pushlog.

> It also does not explain why they are being disabled for webrenderer
only?  I thought you wanted these tests to run so you could reproduce
the slow shutdown times?

I don't care of webrender at the moment, and lots of our user prompt tests are disabled because of webrender bug 1425588. So this is just for conformance with others. Sorry but I don't know the details.

	Andreas Tolfsen ❲:ato❳
	Comment 6 • 6 years ago
mozreview-review-reply

Comment on attachment 8975407 [details]
Bug 1461253 - [wdspec] Disable user prompt tests of get_element_property and get_element_tag_name for webrender.

https://reviewboard.mozilla.org/r/243706/#review249608

> > I think this commit message is way too long.
> 
> There is no restriction to the length and I see way longer commit messages in the pushlog.
> 
> > It also does not explain why they are being disabled for webrenderer
> only?  I thought you wanted these tests to run so you could reproduce
> the slow shutdown times?
> 
> I don't care of webrender at the moment, and lots of our user prompt tests are disabled because of webrender bug 1425588. So this is just for conformance with others. Sorry but I don't know the details.

> There is no restriction to the length and I see way longer commit
> messages in the pushlog.

Established practice is that the first line ideally should not
exceed 50 characters.  Obviously central is a bit special because
we have various tooling that for historical reasons expect a wide
range of metadata to also be present in the first line, so that
recommendation should be taken with a grain of salt.

The fact that there exists commit messages with longer first lines
is not itself an argument that it is good practice.

> I don't care of webrender at the moment, and lots of our user prompt
> tests are disabled because of webrender bug 1425588. So this is
> just for conformance with others. Sorry but I don't know the details.

Thanks, that’s the context I was missing here.

	Andreas Tolfsen ❲:ato❳
	Comment 7 • 6 years ago
mozreview-review

Comment on attachment 8975407 [details]
Bug 1461253 - [wdspec] Disable user prompt tests of get_element_property and get_element_tag_name for webrender.

https://reviewboard.mozilla.org/r/243706/#review249908

	Henrik Skupin [:whimboo][⌚️UTC+1] Assignee
	Comment 8 • 6 years ago
mozreview-review-reply

Comment on attachment 8975407 [details]
Bug 1461253 - [wdspec] Disable user prompt tests of get_element_property and get_element_tag_name for webrender.

https://reviewboard.mozilla.org/r/243706/#review249608

> > There is no restriction to the length and I see way longer commit
> > messages in the pushlog.
> 
> Established practice is that the first line ideally should not
> exceed 50 characters.  Obviously central is a bit special because
> we have various tooling that for historical reasons expect a wide
> range of metadata to also be present in the first line, so that
> recommendation should be taken with a grain of salt.
> 
> The fact that there exists commit messages with longer first lines
> is not itself an argument that it is good practice.
> 
> > I don't care of webrender at the moment, and lots of our user prompt
> > tests are disabled because of webrender bug 1425588. So this is
> > just for conformance with others. Sorry but I don't know the details.
> 
> Thanks, that’s the context I was missing here.

> Established practice is that the first line ideally should not
> exceed 50 characters.  Obviously central is a bit special because

If I would have to reduce the line length useful content will be missing which not necessarily helps in finding a specific code change when going through the pushlog. It's a trade-off and as you know we do already our best to keep it short. But in some cases exception should be possible.

	Pulsebot
	Comment 9 • 6 years ago

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/78df93cfb094
[wdspec] Disable user prompt tests of get_element_property and get_element_tag_name for webrender. r=ato

	Dorel Luca [:dluca]
	Comment 10 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/78df93cfb094