1423667 - Fix and re-enable some tests that were disabled on Windows Code Coverage builds after the landing of clang-cl r317840

Status: RESOLVED FIXED

(Testing :: Code Coverage, enhancement)

Description

[Ryan VanderMeulen [:RyanVM]]

In bug 1423307, we had to update clang-cl to fix some issues with a recent Visual Studio update. Unfortunately, this appears to have caused some new permafails on Windows Code Coverage builds. Per discussion with Marco, we're going to disable the affected tests for now to unblock things.

Comment 1

[Ryan VanderMeulen [:RyanVM]]

FWIW, Places appears to be a common factor in many of the failing tests.

Comment 2

[Marco Castelluccio [:marco]]

It's a pity that so many tests are consistently failing now (with the previous llvm revision, we only had to disable/mark as failing 6 tests), looking at your latest try build at https://treeherder.mozilla.org/#/jobs?repo=try&noautoclassify&revision=bfe9d8401b8e6922238b2e3e65b12e4225a23074&group_state=expanded&filter-tier=1&filter-tier=2&filter-tier=3.

I was hoping some of them would be intermittents, but they aren't.

I see some of the browser-chrome mochitests are failing because of timeouts, perhaps we can fix them simply by increasing the timeouts.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2273112495f
Disable some tests that fail on Windows Code Coverage builds with clang-cl r317840. rs=marco

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Really sorry I ended up having to disable as much as I did :(

I tried to use fail-if as much as possible, but that proved pretty brittle unfortunately. As I mentioned before, a lot of the failures were related to places, so maybe there's an underlying issue that will take care of many of them.

I also wouldn't be surprised if some of the failures in browser/components/places/tests/browser were actually bad state carrying over from previously-failed tests and that some of them can probably be re-enabled. But given the time-sensitivity involved, I didn't really get a chance to explore that much.

Anyway, sorry to leave you in a bit of a lurch.

Comment 5

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/d2273112495f

Comment 6

[Mark Banner (:standard8)]

Marco: do we have any idea what's causing the places failures? It seems to be mainly affecting tests that open the library window

Comment 7

[(Away)]

(In reply to Mark Banner (:standard8) from comment #6)
> Marco: do we have any idea what's causing the places failures? It seems to
> be mainly affecting tests that open the library window

I also see tons of places failures in non-ccov clang-cl builds, but only on 64-bit opt. I betcha it's some stupid "we broke the rules of C++ in a way that MSVC happily accepted". It's on my list to look at today.

Comment 8

[(Away)]

Oof, the places stuff is all frontend code. I'm probably not going to be very effective at debugging this.

Comment 9

[(Away)]

The places tests are green after I pick up the clang fix in bug 1448519.

Comment 10

[Marco Castelluccio [:marco]]

(In reply to David Major [:dmajor] from comment #9)
> The places tests are green after I pick up the clang fix in bug 1448519.

Did you try re-enabling all the disabled tests when you picked up the clang fix? Were they all still failing except the places ones?

Comment 11

[(Away)]

No, I haven't looked through the remaining ccov annotations.

Comment 12

[Marco Castelluccio [:marco]]

Attachment: Re-enable tests

They are all passing now.

Comment 13

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8965362 [details] [diff] [review]
Re-enable tests

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

excellent.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6e53256c925
Reenable some tests that were disabled because of a Clang update. r=jmaher

Comment 15

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/b6e53256c925