golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.7k stars 17.49k forks source link

x/mobile/cmd/gomobile: the test TestWriter is flaky on the trybots #40290

Open hajimehoshi opened 4 years ago

hajimehoshi commented 4 years ago

I often see the test failure on the try bots:

--- FAIL: TestWriter (0.02s)
    writer_test.go:75: unexpected output from aapt
    writer_test.go:80: --- /workdir/tmp/gofmt496954462  2020-07-19 04:40:24.154982542 +0000
        +++ /workdir/tmp/gofmt618424357 2020-07-19 04:40:24.154982542 +0000
        @@ -1,3 +1,4 @@
        +W/ResourceType(19449): Bad XML block: header size 24941 or total size 1701210478 is larger than data size 586
         AndroidManifest.xml
         assets/hello_world.txt
         META-INF/MANIFEST.MF
        @@ -8,23 +9,3 @@
         Package Groups (0)

         Android manifest:
        -N: android=http://schemas.android.com/apk/res/android
        -  E: manifest (line=2)
        -    A: package="org.golang.fakeapp" (Raw: "org.golang.fakeapp")
        -    A: android:versionCode(0x0101021b)=(type 0x10)0x1
        -    A: android:versionName(0x0101021c)="1.0" (Raw: "1.0")
        -    E: uses-sdk (line=8)
        -      A: android:minSdkVersion(0x0101020c)=(type 0x10)0xf
        -    E: application (line=9)
        -      A: android:label(0x01010001)="FakeApp" (Raw: "FakeApp")
        -      A: android:hasCode(0x0101000c)=(type 0x12)0x0
        -      A: android:debuggable(0x0101000f)=(type 0x12)0xffffffff
        -      E: activity (line=10)
        -        A: android:name(0x01010003)="android.app.NativeActivity" (Raw: "android.app.NativeActivity")
        -        A: android:label(0x01010001)="FakeApp" (Raw: "FakeApp")
        -        A: android:configChanges(0x0101001f)=(type 0x11)0xa0
        -        E: intent-filter (line=14)
        -          E: action (line=15)
        -            A: android:name(0x01010003)="android.intent.action.MAIN" (Raw: "android.intent.action.MAIN")
        -          E: category (line=16)
        -            A: android:name(0x01010003)="android.intent.category.LAUNCHER" (Raw: "android.intent.category.LAUNCHER")
FAIL
FAIL    golang.org/x/mobile/cmd/gomobile    147.577s

Is there an issue in the test itself or in the trybots? Can we skip this test as a tentative solution?

CC @hyangah @dmitshur

gopherbot commented 4 years ago

Change https://golang.org/cl/243839 mentions this issue: cmd/gomobile: skip TestWriter

dmitshur commented 4 years ago

Do you know since when it became flaky? How often have you seen it?

I'm not seeing many (or any) previous failures on the post-submit runs (https://build.golang.org/?repo=golang.org%2fx%2fmobile). Is the test only flaky on pre-submit runs?

hajimehoshi commented 4 years ago

Is the test only flaky on pre-submit runs?

I've seen a successful case, which was rare though.

https://go-review.googlesource.com/c/mobile/+/241717/4#message-f6089a23a2d84bb4cb39be3732e730b6704b12f7

I'm not sure whether this issue is only on pre-submit or not.

hajimehoshi commented 4 years ago

Do you know since when it became flaky?

No, I don't... As of May or July, maybe?

dmitshur commented 4 years ago

Oh, I was misreading the build dashboard. The linux-amd64-androidemu builder is configured run pre-submit trybots, but not post-submit builds:

image

Skipping it with this issue open seems reasonable to me, but let's make the skip conditional on it being the linux-amd64-androidemu builder, so it's not skipped unnecessarily elsewhere (unless there's a need for it to be skipped elsewhere as well). I'll post this as a comment on the CL.

dmitshur commented 4 years ago

It turns out it's a bug in the dashboard UI rather than intentional configuration. The linux-amd64-androidemu builder is configured to run both on pre- and post-submit builders. There's even a test case enforcing it. I'll look into fixing the UI so we can see how long this has been failing.

dmitshur commented 4 years ago

a bug in the dashboard UI

I've looked into it, it's not a trivial mistake, the problem happens due to insufficient data and a more complete fix would need changes to multiple components (the build dashboard and maintner API server). That'll take longer and it'll happen as part of other work to improve the dashboard UI (see #34744, #28643, etc.). /cc @toothrot

For now, I've sent a smaller patch in CL 244137 so that existing builds aren't hidden, and consider the rest as future work.

It can be previewed to see the past TestWriter failures here.

gopherbot commented 4 years ago

Change https://golang.org/cl/244137 mentions this issue: app/appengine: stop hiding some tested configurations for x repos