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

Hard to trace behavior change that causes tests to fail when bumping to Go 1.19 from Go 1.18 #55009

Closed
TimothyStiles opened this issue Sep 12, 2022 · 2 comments · Fixed by bebop/poly#265

Comments

@TimothyStiles
Copy link

What version of Go are you using (go version)?

$ go version 1.18 and 1.19

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
sed: illegal option -- r usage: sed script [-Ealn] [-i extension] [file ...] sed [-Ealn] [-i extension] [-e script] ... [-f script_file] ... [file ...] GO111MODULE="" GOARCH="amd64" GOBIN="" GOCACHE="/Users/timothystiles/Library/Caches/go-build" GOENV="/Users/timothystiles/Library/Application Support/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="amd64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/timothystiles/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/timothystiles/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/Users/timothystiles/sdk/go1.18beta1" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/Users/timothystiles/sdk/go1.18beta1/pkg/tool/darwin_amd64" GOVCS="" GOVERSION="go1.18beta1" GCCGO="gccgo" GOAMD64="v1" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/timothystiles/git/poly/go.mod" GOWORK="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/xl/y4w0nnw1621_9ngxx_l6bwn80000gn/T/go-build3864710302=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Switched to Go 1.19 and ran this test.

https://github.com/TimothyStiles/poly/blob/eba6b72e97c66f6e8a15016710b73a2f93a027c3/synthesis/fix/example_test.go#L31

What did you expect to see?

The test passing the same way that it has in 1.18 and previous versions of Go.

What did you see instead?

The test fails with a two character difference from its expected output

Screen_Shot_2022-09-11_at_4 24 49_PM

Screen Shot 2022-09-11 at 2 17 43 PM

I'm stuck on something pretty tricky and am hoping someone here knows something about this.

This example test passes when using Go 1.18 and fails when using Go 1.19.

https://github.com/TimothyStiles/poly/blob/eba6b72e97c66f6e8a15016710b73a2f93a027c3/synthesis/fix/example_test.go#L31

The functions involved are stochastic and its output should never change but when run with Go 1.19 I get the errors that I've attached above.

Does anyone have any idea what's going on here? Maybe some unexpected behavior change in unmarshaling JSON between the two versions?

Thanks,
Tim

@Jorropo
Copy link
Member

Jorropo commented Sep 12, 2022

Bisected to 72e77a7 (sort: use pdqsort).

My 2 blind guesses are (in order of how realistic I think this is):

  1. you sort a list with some different elements of equal value, and for whatever reason pdqsort order them differently than previously (if that true using the *Stable sorts would solve this, actually you might rely on the old previous unstable ordering however that just broken code if it's the case).
  2. you somewhere do a sort but there is some side effects in the compare or swap function, because pdqsort has a different pattern it has different side effects.

A bug in the sort implementation is possible obviously but I would find that unlikely given how well it's tested.

Edit:
Looking at 72e77a7 I see that some tests were broken precisely for this reason, so this is a behaviour known of this new implementation (tl;dr unstable equal order changed and you were probably relying on it implicitly).

Jorropo added a commit to Jorropo/poly that referenced this issue Sep 12, 2022
TimothyStiles added a commit to bebop/poly that referenced this issue Sep 12, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
* fix: Use SliceStable to sort potential changes

Fixes golang/go#55009

Co-authored-by: Tim <TimothyStiles@users.noreply.github.com>
@TimothyStiles
Copy link
Author

Bisected to 72e77a7 (sort: use pdqsort).

My 2 blind guesses are (in order of how realistic I think this is):

1. you sort a list with some different elements of equal value, and for whatever reason pdqsort order them differently than previously (if that true using the `*Stable` sorts would solve this, actually you might rely on the old previous unstable ordering however that just broken code if it's the case).

2. you somewhere do a sort but there is some side effects in the compare or swap function, because pdqsort has a different pattern it has different side effects.

A bug in the sort implementation is possible obviously but I would find that unlikely given how well it's tested.

Edit: Looking at 72e77a7 I see that some tests were broken precisely for this reason, so this is a behaviour known of this new implementation (tl;dr unstable equal order changed and you were probably relying on it implicitly).

This was exactly the case @Jorropo! Thanks so much for making a PR!

-Tim

@golang golang locked and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants