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

Update module mitchellh/mapstructure to 1.5.0 #2111

Merged
merged 2 commits into from Sep 21, 2022
Merged

Conversation

StevenACoffman
Copy link
Collaborator

@StevenACoffman StevenACoffman commented Apr 20, 2022

Updating all module dependencies to the latest version causes a few WebSocket tests to fail. I wonder if @telemenar or @RobinCPel knows why updating mitchellh/mapstructure to 1.5.0 seems to break them.

The mapstructure package can be updated to v1.3.1, any other version - from v1.3.2 and up - seems to break the test...

Signed-off-by: Steve Coffman steve@khanacademy.org

@StevenACoffman
Copy link
Collaborator Author

@wilhelmeek If you have a chance to look at this, you have more experience with this than I do.

@StevenACoffman StevenACoffman added the invalid This doesn't seem right label Apr 20, 2022
@StevenACoffman StevenACoffman marked this pull request as draft April 20, 2022 19:04
@RobinCPel
Copy link
Contributor

RobinCPel commented Apr 21, 2022

If I update all of the go mods except for github.com/mitchellh/mapstructure, go test -race ./... succeeds. But when I update it as well, the test fails. 🤔

After some more trial-and-error: The mapstructure package can be updated to v1.3.1, any other version - from v1.3.2 and up - seems to break the test...

For reference: mapstructure's changelog.

@StevenACoffman
Copy link
Collaborator Author

StevenACoffman commented Apr 24, 2022

Thanks to @RobinCPel I was able to upgrade the rest and merged that as a separate PR, and I rebased this so it just shows the specific problem with this one library version. I'm not able to spend more time digging into what needs to change, but we'll need to wrestle with this eventually.

@StevenACoffman StevenACoffman changed the title Update modules Update module mitchellh/mapstructure to 1.5.0 Apr 24, 2022
@StevenACoffman StevenACoffman added the help wanted Extra attention is needed label Apr 24, 2022
@ubiuser
Copy link
Contributor

ubiuser commented May 30, 2022

I hope this helps to find the answer: mitchellh/mapstructure#290

Signed-off-by: Steve Coffman <steve@khanacademy.org>
@coveralls
Copy link

coveralls commented Sep 21, 2022

Coverage Status

Coverage decreased (-0.006%) to 75.458% when pulling 9fd55d1 on update_modules into ea9590a on master.

Signed-off-by: Steve Coffman <steve@khanacademy.org>
@StevenACoffman
Copy link
Collaborator Author

It looks like we were always taking the pointer of a pointer in the websocket code, and somehow this worked ok in the old mapstructure, but not in the newer versions.

return sock.Next(&resp)

Special thanks to @jakezhu9 and @ubiuser who helped figure this out in mitchellh/mapstructure#290 (comment)

@StevenACoffman StevenACoffman merged commit 56f6db0 into master Sep 21, 2022
@StevenACoffman StevenACoffman deleted the update_modules branch September 21, 2022 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants