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

RESOLVED FIXED

Status

enhancement
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: RyanVM, Assigned: marco)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

a year ago
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.
(Reporter)

Comment 1

a year ago
FWIW, Places appears to be a common factor in many of the failing tests.
(Assignee)

Comment 2

a year ago
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

a year ago
Pushed by ryanvm@gmail.com:
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
(Reporter)

Comment 4

a year ago
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.
(Assignee)

Updated

a year ago
See Also: → 1444591
Depends on: 1445652
Marco: do we have any idea what's causing the places failures? It seems to be mainly affecting tests that open the library window
Flags: needinfo?(mcastelluccio)
(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.
Oof, the places stuff is all frontend code. I'm probably not going to be very effective at debugging this.
The places tests are green after I pick up the clang fix in bug 1448519.
Depends on: 1448519
Flags: needinfo?(mcastelluccio)
Depends on: 1451479
(Assignee)

Comment 10

a year ago
(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?
Flags: needinfo?(dmajor)
No, I haven't looked through the remaining ccov annotations.
Flags: needinfo?(dmajor)
(Assignee)

Comment 12

a year ago
They are all passing now.
Assignee: nobody → mcastelluccio
Status: NEW → ASSIGNED
Attachment #8965362 - Flags: review?(jmaher)
Comment on attachment 8965362 [details] [diff] [review]
Re-enable tests

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

excellent.
Attachment #8965362 - Flags: review?(jmaher) → review+

Comment 14

a year ago
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6e53256c925
Reenable some tests that were disabled because of a Clang update. r=jmaher
(Assignee)

Updated

a year ago
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.