Un-hide Gaia Build Unit tests when they meet visibility standards

RESOLVED FIXED

Status

Tree Management
Visibility Requests
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: RyanVM, Assigned: rickychien)

Tracking

Trunk
x86
Windows 8.1
Dependency tree / graph

Details

Attachments

(2 attachments)

Not only are they currently perma-failing (tracked in bug 1059447), but the failures also provide no insight into what part of the test is actually failing. Looking at the test in question, I see multiple asserts in each sub-test. The harness should be saying what assert is actually failing so that developers can realistically debug any failures they hit.

Comment 1

4 years ago
Few more need doing (using script in bug 1054228): cedar/pine
Product: Webtools → Tree Management

Updated

3 years ago
Component: TBPL → Treeherder
(Assignee)

Comment 2

3 years ago
I marked bug 1059447 as resolved and I think Gbu is stable for a few months ago. It's time to un-hide it!
Flags: needinfo?(ryanvm)
Being stable isn't all that matters here. Please verify the answers to the below points from the Job Visibility Policy. More information on each point is provided at the link below.
https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy

1.1 Has an active owner
1.2 Usable job logs
1.3 Low intermittent failure rate
1.4 Must avoid patterns known to cause non deterministic failures
1.5 Has sufficient documentation
2.1 Breakage is expected to be followed by tree closure or backout
2.2 Runs on mozilla-central and all trees that merge into it
2.3 Scheduled on every push
2.4 Easily run on try server

Not required for default visibility, but good to know nonetheless:
3.1 Easy for a dev to run locally
3.2 Supports the disabling of individual tests

Thanks!
Flags: needinfo?(ryanvm)
(Assignee)

Comment 4

3 years ago
I'm not a sheriff and don't realize the policy you mentioned, so I wonder who's the best person to ensure these a bunch of items. Do you know who can help us?
Flags: needinfo?(ryanvm)
Whoever owns these tests should be the one who's answering them. I don't know who that is within the FxOS organization.
Flags: needinfo?(ryanvm)
(Assignee)

Comment 6

3 years ago
1.1 Has an active owner - (George and I are active peers)
1.2 Usable job logs - ok
1.3 Low intermittent failure rate - ok
1.4 Must avoid patterns known to cause non deterministic failures - ok
1.5 Has sufficient documentation - ok
2.1 Breakage is expected to be followed by tree closure or backout - ok
2.2 Runs on mozilla-central and all trees that merge into it - ok
2.3 Scheduled on every push - ok
2.4 Easily run on try server - ok

Gbu was un-hide in the past, so it has passed above items.
Flags: needinfo?(ryanvm)
(In reply to Ricky Chien [:rickychien] from comment #6)
> 1.1 Has an active owner - (George and I are active peers)

OK

> 1.2 Usable job logs - ok

See comment 0 in this bug. Has that been resolved yet?

> 1.3 Low intermittent failure rate - ok

OK

> 1.4 Must avoid patterns known to cause non deterministic failures - ok

The page I linked to specifically references not touching external servers and having MOZ_DISABLE_NONLOCAL_CONNECTIONS set. Can you please point to where that's set for Gb/Gbu?

> 1.5 Has sufficient documentation - ok

Link?

> 2.1 Breakage is expected to be followed by tree closure or backout - ok
> 2.2 Runs on mozilla-central and all trees that merge into it - ok
> 2.3 Scheduled on every push - ok
> 2.4 Easily run on try server - ok

OK

> Gbu was un-hide in the past, so it has passed above items.

I don't think that's a good assumption to make.
Flags: needinfo?(ryanvm)
(Assignee)

Comment 8

3 years ago
1.2 Usable job logs

We can see Gbu AssertionError details in Gaia-try now.

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=6e0c2d03c82f
(Assignee)

Comment 9

3 years ago
Created attachment 8536444 [details] [review]
Gaia PR for doc

Add documentation for how to run build system test.
Attachment #8536444 - Flags: review?(gduan)
Comment on attachment 8536444 [details] [review]
Gaia PR for doc

r=gduan
Attachment #8536444 - Flags: review?(gduan) → review+
I read doc from [1] but still cannot understand your question in question 1.4 on comment 7.

Could you tell me more details? thanks!

[1] https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#Must_avoid_patterns_known_to_cause_non_deterministic_failures
Flags: needinfo?(ryanvm)
There are intermittents on file for this suite where these tests fail due to Github being unavailable. Our test suites should not be touching external infrastructure for exactly that reason. To aid in that process, we have a MOZ_DISABLE_NONLOCAL_CONNECTIONS environment variable that causes a crash when an attempt to reach an external site is made. It is set for most of our test suites at this point, but the fact that Gb connects to Github tells me that it isn't for these suites.

You need to fix the suites so they aren't connecting to external sites and make sure MOZ_DISABLE_NONLOCAL_CONNECTIONS is being used when they run to avoid regressing it in the future.

Make sense?
Flags: needinfo?(ryanvm)
Created attachment 8538966 [details] [review]
Gaia PR for MOZ_DISABLE_NONLOCAL_CONNECTIONS

Enable MOZ_DISABLE_NONLOCAL_CONNECTIONS=1
Attachment #8538966 - Flags: review?(gduan)
(Assignee)

Updated

3 years ago
Assignee: nobody → ricky060709
Status: NEW → ASSIGNED
Comment on attachment 8538966 [details] [review]
Gaia PR for MOZ_DISABLE_NONLOCAL_CONNECTIONS

Hi Ryan,
I don't know if only setting MOZ_DISABLE_NONLOCAL_CONNECTIONS env could prevent what you mentioned in comment 13. Perhaps we should also bring bug 1024047 which should solve intermittent failure into this commit.
Attachment #8538966 - Flags: feedback?(ryanvm)
Merged Add MOZ_DISABLE_NONLOCAL_CONNECTIONS for build tests.

https://github.com/mozilla-b2g/gaia/commit/4460619f8901212315201b12727c2bf974889f06
Comment on attachment 8538966 [details] [review]
Gaia PR for MOZ_DISABLE_NONLOCAL_CONNECTIONS

(In reply to George Duan [:gduan] [:喬智] from comment #15)
> I don't know if only setting MOZ_DISABLE_NONLOCAL_CONNECTIONS env could
> prevent what you mentioned in comment 13. Perhaps we should also bring bug
> 1024047 which should solve intermittent failure into this commit.

I have to say, I'm confused as to how this is passing post-merge if these tests are still touching github. But yeah, we should definitely get bug 1024047 fixed one way or another.
Attachment #8538966 - Flags: feedback?(ryanvm)
The define only makes _gecko_ code crash if making non-local connections, not harness code, unless it is also making use of say xpcshell. As such scripting in automation outside of this will slip past.
Seems that there are some intermittent Gb failures after adding MOZ_DISABLE_NONLOCAL_CONNECTIONS was landed.

I'm going to fix bug 1024047 as soon as possible and hope that issue can be solved.
(Assignee)

Updated

3 years ago
Depends on: 1024047
(Assignee)

Updated

3 years ago
No longer depends on: 1024047
Comment on attachment 8538966 [details] [review]
Gaia PR for MOZ_DISABLE_NONLOCAL_CONNECTIONS

r+ it, it seems that you've already merged it.
Attachment #8538966 - Flags: review?(gduan) → review+

Updated

3 years ago
Component: Treeherder → Visibility Requests
(Assignee)

Updated

3 years ago
Depends on: 1024047
(Assignee)

Updated

3 years ago
Depends on: 1072842
> 1.4 Must avoid patterns known to cause non deterministic failures

Patch has been landed! Through TRY_ENV=1 to set MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 in [1]. I already patched mozharness to setup TRY_ENV=1 when executing Gbu and Gb [2].

[1] https://github.com/RickyChien/gaia/blob/865e8430ecc1543472fd27d165a95fce76d6a651/build/build-test.js#L31-L34
[2] https://hg.mozilla.org/build/mozharness/rev/020094345d6c


> 1.5 Has sufficient documentation

https://github.com/RickyChien/gaia/tree/865e8430ecc1543472fd27d165a95fce76d6a651#build-system-tests
Flags: needinfo?(ryanvm)
(In reply to Ricky Chien [:rickychien] from comment #22)
> > 1.4 Must avoid patterns known to cause non deterministic failures
> 
> Patch has been landed! Through TRY_ENV=1 to set
> MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 in [1]. I already patched mozharness to
> setup TRY_ENV=1 when executing Gbu and Gb [2].

Setup TRY_ENV=1 will rule out all network required tests. So it would be more stable than today.
MOZ_DISABLE_NONLOCAL_CONNECTIONS=1 has enabled both in Gbu and Gb currently. So we can un-hide Gb in bug 1059455 once this bug is resolved.
Looks great, thanks for sticking with this! I've looked through the recent results and Gb/Gbu looks green, so I've gone ahead and made them visible again.

One question, though. Do any of your harness fixes need backporting to older Gaia branches, i.e. v2.0/v2.1?
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(ryanvm) → needinfo?(ricky060709)
Resolution: --- → FIXED
No, we don't need to do that. thanks!
Flags: needinfo?(ricky060709)
You need to log in before you can comment on or make changes to this bug.