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

Status: RESOLVED FIXED

(Tree Management Graveyard :: Visibility Requests, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

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

[Ed Morley [:emorley]]

Few more need doing (using script in bug 1054228): cedar/pine

Comment 2

[Ricky Chien [:rickychien] (inactive)]

I marked bug 1059447 as resolved and I think Gbu is stable for a few months ago. It's time to un-hide it!

Comment 3

[Ryan VanderMeulen [: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!

Comment 4

[Ricky Chien [:rickychien] (inactive)]

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?

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Whoever owns these tests should be the one who's answering them. I don't know who that is within the FxOS organization.

Comment 6

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 7

[Ryan VanderMeulen [: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.

Comment 8

[Ricky Chien [:rickychien] (inactive)]

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

Comment 9

[Ricky Chien [:rickychien] (inactive)]

Attachment: Gaia PR for doc

Add documentation for how to run build system test.

Comment 10

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8536444 [details] [review]
Gaia PR for doc

r=gduan

Comment 11

[Ricky Chien [:rickychien] (inactive)]

Merged Gaia PR for doc!

https://github.com/mozilla-b2g/gaia/commit/ec262f794e9d811619e24d2e17ccdb4ffa41f808

Comment 12

[Ricky Chien [:rickychien] (inactive)]

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

Comment 13

[Ryan VanderMeulen [: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?

Comment 14

[Ricky Chien [:rickychien] (inactive)]

Attachment: Gaia PR for MOZ_DISABLE_NONLOCAL_CONNECTIONS

Enable MOZ_DISABLE_NONLOCAL_CONNECTIONS=1

Comment 15

[(Inactive after June) George Duan [:gduan] [:喬智]]

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.

Comment 16

[Ricky Chien [:rickychien] (inactive)]

Try status:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7aadf3fa2d6

Comment 17

[Ricky Chien [:rickychien] (inactive)]

Merged Add MOZ_DISABLE_NONLOCAL_CONNECTIONS for build tests.

https://github.com/mozilla-b2g/gaia/commit/4460619f8901212315201b12727c2bf974889f06

Comment 18

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 19

[Ed Morley [:emorley]]

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.

Comment 20

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 21

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8538966 [details] [review]
Gaia PR for MOZ_DISABLE_NONLOCAL_CONNECTIONS

r+ it, it seems that you've already merged it.

Comment 22

[Ricky Chien [:rickychien] (inactive)]

> 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

Comment 23

[Ricky Chien [:rickychien] (inactive)]

(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.

Comment 24

[Ricky Chien [:rickychien] (inactive)]

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.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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?

Comment 26

[Ricky Chien [:rickychien] (inactive)]

No, we don't need to do that. thanks!