x/mobile/bind: TestGenGo failing due to changes in gofmt output #34619
Comments
bcmills
added
the
Soon
|
Change https://golang.org/cl/198038 mentions this issue: |
|
Rewriting the tests to do this in a more robust way is a bit tricky due to the way they are structured. In particular For example, https://github.com/golang/mobile/blob/master/bind/testdata/java.go.golden has multiple packages in it. We can't just run this file through the formatter because The test generates some go code without gofmt-ing it first, which includes multiple packages (in |
|
A couple of possible solutions:
|
|
I will send a cl soon. It is a major change though since the reverse
binding doesn't do formatting, it turned out.
…On Tue, Oct 1, 2019, 1:27 PM Eli Bendersky ***@***.***> wrote:
A couple of possible solutions:
1.
Hacky but relatively simple: when reading the golden file, find package
main and only reformat that package, leaving the rest of the code
unformatted, as today. package main comes at the end of the file, so
it's easy.
2.
More robust, but entails a significant restructuring of bind_test:
don't generate packages into the same file with the main Go code. Generate
them (genJavaPackages) into a separate golden file, that gets compared
separately. So instead of a single golden file per test, there will be two
- and the two are compared separately. The one where the main Go code is
generated will pass through format.Source for stability.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#34619?email_source=notifications&email_token=ABGESL5L34DR2I3TBDM6IRDQMOB6RA5CNFSM4I37KH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEACCASA#issuecomment-537141320>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABGESL2OJTBZOHF4USCJ4E3QMOB6RANCNFSM4I37KH4Q>
.
|
|
Change https://golang.org/cl/198322 mentions this issue: |
bind_test.go compares the generated Go files against golden files checked in the repository. The bind package formats some of the generated Go files, so any changes in the go formatter can break the tests. This change makes the test more robust by applying formatting based on the currently used go version. Since a golden file often includes multiple go files generated by the bind, the `gofmt` function splits the golden file using the gobindPreamble marker and then run format.Source for each chunk. In order to ease the golden file splitting, this CL also moves the gobindPreamble to the beginning of each generated file consistently. It turned out bind omits formatting for some go files (generated for reverse binding). That needs to be fixed but it is a much bigger fix. Thus, in this CL, we apply the formatting on the bind's output as well. This CL also updates the gobindPreamble to follow the style guide for generated code. https://golang.org/s/generatedcode Fixes golang/go#34619 Change-Id: Ia2957693154face2848e051ebbb2373e95d79593 Reviewed-on: https://go-review.googlesource.com/c/mobile/+/198322 Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
bind_test.go compares the generated Go files against golden files checked in the repository. The bind package formats some of the generated Go files, so any changes in the go formatter can break the tests. This change makes the test more robust by applying formatting based on the currently used go version. Since a golden file often includes multiple go files generated by the bind, the `gofmt` function splits the golden file using the gobindPreamble marker and then run format.Source for each chunk. In order to ease the golden file splitting, this CL also moves the gobindPreamble to the beginning of each generated file consistently. It turned out bind omits formatting for some go files (generated for reverse binding). That needs to be fixed but it is a much bigger fix. Thus, in this CL, we apply the formatting on the bind's output as well. This CL also updates the gobindPreamble to follow the style guide for generated code. https://golang.org/s/generatedcode Fixes golang/go#34619 Change-Id: Ia2957693154face2848e051ebbb2373e95d79593 Reviewed-on: https://go-review.googlesource.com/c/mobile/+/198322 Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
The fix for #28082 changed the line-wrapping behavior of
gofmtin some cases.One of those changes happens to affect
x/mobile/bind.TestGenGo, which is asserting a particular goldengofmtoutput (rather than more robustly running both the actual and expected outputs throughgofmtbefore comparing them).CC @griesemer @eliben @odeke-em @steeve @hyangah
The text was updated successfully, but these errors were encountered: