Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
hajimehoshi opened this issue Jul 19, 2020 · 8 comments
Open

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

hajimehoshi opened this issue Jul 19, 2020 · 8 comments
Labels
mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@hajimehoshi
Copy link
Member

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 gopherbot added the mobile Android, iOS, and x/mobile label Jul 19, 2020
@gopherbot gopherbot added this to the Unreleased milestone Jul 19, 2020
@gopherbot
Copy link

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

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 21, 2020
@dmitshur
Copy link
Contributor

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
Copy link
Member Author

hajimehoshi commented Jul 21, 2020 •
edited

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
Copy link
Member Author

Do you know since when it became flaky?

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

@dmitshur
Copy link
Contributor

dmitshur commented Jul 21, 2020 •
edited

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
Copy link
Contributor

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
Copy link
Contributor

dmitshur commented Jul 22, 2020 •
edited

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
Copy link

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

gopherbot pushed a commit to golang/build that referenced this issue Jul 22, 2020
@dmitshur
app/appengine: stop hiding some tested configurations for x repos
70303fc 
The build dashboard can be used to view builds for the main Go repo
and other golang.org/x repos. The isUntested invocation was invalid
for repos other than the main one, which was causing results for some
tested configurations to become hidden (incorrectly replaced by '•').

Larger changes are needed before there's sufficient data to compute
the GoBranch value for all repos reliably, so for now, just update
the isUntested invocation to apply only for the main repo.

This prioritizes the ability to view test results over the ability
to see that some builds are intentionally missing because they are
configured not to run. Doing both is a part of future work.

For golang/go#40290.
For golang/go#34744.
For golang/go#28643.

Change-Id: Id43cb47abacb1036f578efbb8232ae17ad40eca9
Reviewed-on: https://go-review.googlesource.com/c/build/+/244137
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Reviewed-by: Carlos Amedee <carlos@golang.org>
gopherbot pushed a commit to golang/mobile that referenced this issue Aug 1, 2020
@hajimehoshi
cmd/gomobile: skip TestWriter
973feb4 
TestWriter is flaky and often fails on the trybots. Skip this as a
tentative solution.

Updates golang/go#40290

Change-Id: I3a8aa74fb6cb727a216da4046edaa159f9aa2dc3
Reviewed-on: https://go-review.googlesource.com/c/mobile/+/243839
Run-TryBot: Hajime Hoshi <hajimehoshi@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
roderm pushed a commit to roderm/mobile that referenced this issue Sep 13, 2020
@hajimehoshi
cmd/gomobile: skip TestWriter
259bfeb 
TestWriter is flaky and often fails on the trybots. Skip this as a
tentative solution.

Updates golang/go#40290

Change-Id: I3a8aa74fb6cb727a216da4046edaa159f9aa2dc3
Reviewed-on: https://go-review.googlesource.com/c/mobile/+/243839
Run-TryBot: Hajime Hoshi <hajimehoshi@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 10, 2021
@hajimehoshi
cmd/gomobile: skip TestWriter
11782f6 
TestWriter is flaky and often fails on the trybots. Skip this as a
tentative solution.

Updates golang/go#40290

Change-Id: I3a8aa74fb6cb727a216da4046edaa159f9aa2dc3
Reviewed-on: https://go-review.googlesource.com/c/mobile/+/243839
Run-TryBot: Hajime Hoshi <hajimehoshi@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 11, 2021
@hajimehoshi
cmd/gomobile: skip TestWriter
ab94964 
TestWriter is flaky and often fails on the trybots. Skip this as a
tentative solution.

Updates golang/go#40290

Change-Id: I3a8aa74fb6cb727a216da4046edaa159f9aa2dc3
Reviewed-on: https://go-review.googlesource.com/c/mobile/+/243839
Run-TryBot: Hajime Hoshi <hajimehoshi@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@hajimehoshi @dmitshur @gopherbot