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

many: fix tests with go1.8 / artful #3250

Merged
merged 6 commits into from Apr 28, 2017
Merged

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Apr 28, 2017

Also run new "artful" autopkgtest hook that should ensure we run not into failures for this anymore.

This will also need to be distro-pachted into the artful package to make the build (or our SRU will be blocked on this).

Copy link
Collaborator

@zyga zyga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what's going on but +1

@mvo5 mvo5 changed the title store: fix TestDownloadSyncFails for go1.8 store: fix tests with go1.8 Apr 28, 2017
Copy link
Collaborator

@zyga zyga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for iterating on those fixes

c.Check(all, Contains, &builtin.UhidInterface{})
c.Check(all, Contains, &builtin.Unity7Interface{})
c.Check(all, DeepContains, &builtin.BluezInterface{})
c.Check(all, DeepContains, &builtin.BoolFileInterface{})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now I really wonder how this was working before

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see two possibilities:

  • compiler optimization resulted in the same pointer being returned for both
  • check actually handing pointers to simple structures the same way as it would if the structure is passed by value

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope for 1 (bad on go side), I might try to double-check that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here :)

Copy link
Collaborator

@pedronis pedronis Apr 28, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's 1 and something has changed, it's important to note that those structs are zero-sized.

The langref has this to say (at the very end):

"Two distinct zero-size variables may have the same address in memory."

may not must, so both <1.8 and 1.8 are doing acceptable things but the test was broken

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O-M-G that is utterly terrible. I'm happy that they changed this in 1.8.

@mvo5 mvo5 changed the title store: fix tests with go1.8 many: fix tests with go1.8 / artful Apr 28, 2017
Copy link
Collaborator

@zyga zyga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy-paste bug

spread.yaml Outdated
- ubuntu-17.10-ppc64el:
username: ubuntu
password: ubuntu
- ubuntu-17.04-armhf:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a copy-paste bug, I think you wanted 17.10-armhf

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
@zyga zyga merged commit 7611618 into snapcore:master Apr 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants