tests fail with gccgo

Bug #1275410 reported by Michael Hudson-Doyle
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
goyaml
New
Undecided
Gustavo Niemeyer

Bug Description

Like so:

----------------------------------------------------------------------
FAIL: encode_test.go:217: net_goyaml_test.TestMarshal.pN27_launchpad.net_goyaml_test.S

encode_test.go:221:
    c.Assert(string(data), Equals, item.data)
... obtained string = "" +
... "a: 1\n" +
... "b: 2\n"
... expected string = "" +
... "a: 1\n" +
... "b: 2\n" +
... "c: 3\n"

----------------------------------------------------------------------
FAIL: encode_test.go:238: net_goyaml_test.TestMarshalErrors.pN27_launchpad.net_goyaml_test.S

encode_test.go:241:
    c.Assert(err, ErrorMatches, item.error)
... value = nil
... regex string = "Duplicated key 'b' in struct struct \\{ B int; .*"
... Error value is nil

----------------------------------------------------------------------
FAIL: decode_test.go:364: net_goyaml_test.TestUnmarshal.pN27_launchpad.net_goyaml_test.S

decode_test.go:385:
    c.Assert(value, DeepEquals, item.value, Commentf("Item #%d", i))
... obtained *struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" } = &struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" }{A:1, C:goyaml_test.inlineB{B:2, inlineC:goyaml_test.inlineC{C:0}}}
... expected *struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" } = &struct { A int; C goyaml_test.inlineB "yaml:\",inline\"" }{A:1, C:goyaml_test.inlineB{B:2, inlineC:goyaml_test.inlineC{C:3}}}
... Item #84

OOPS: 9 passed, 3 FAILED
--- FAIL: Test (0.02 seconds)
FAIL
FAIL launchpad.net/goyaml 0.025s

Tags: ppc64el
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

All failures seem to be something going wrong with ,inline.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

it's a difference in how reflection handles unexported, inlined, struct fields. I filed a bug upstream: https://code.google.com/p/go/issues/detail?id=7247

I'm not sure which implementation gets it right!

tags: added: ppc64el
Revision history for this message
Mark Ramm (mark-ramm) wrote :

This is marked as fixed in https://code.google.com/p/go/issues/detail?id=7247 can we test against that and see if all is now well?

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Well, all is now well in the sense that the tests fail with go tip too; this bug report is indicating that the goyaml tests depended on a bug in the gc toolchain, which is what has been fixed.

I've just verified that the branch I proposed (now linked to this bug, sorry for not doing this before) fixes the tests on go tip as well as gccgo.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Thanks Michael, I'll get that merged.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Actually, I'm not quite sure that this is right. Exported fields of a non-exported anonymous field are in fact exported too. The test change simply makes everything public, ignoring this case.

I believe the original test should continue to work.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Revision history for this message
Mark Ramm (mark-ramm) wrote :

What's the next step on this?

What needs to happen to make the tests pass under GCC Go. If we get the right direction on this, I'm sure somebody from core can work on it -- but it would be great if I didn't have to pull them away from the many other issues that need attention for GCC to work to get this addressed.

Changed in goyaml:
assignee: nobody → Gustavo Niemeyer (niemeyer)
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

This will likely get fixed upstream. The change that went it to get gc and gccgo to work the same broke the behavior not only of goyaml, but also of the stock encoding/json package, and likely many other marshaling packages out there. There's already partial agreement from the core team that this is not okay and must be addressed before Go 1.3.

Revision history for this message
Mark Ramm (mark-ramm) wrote : Re: [Bug 1275410] Re: tests fail with gccgo

Awesome. Thanks for the update.

On Thu, Mar 13, 2014 at 1:29 PM, Gustavo Niemeyer <email address hidden>wrote:

> This will likely get fixed upstream. The change that went it to get gc
> and gccgo to work the same broke the behavior not only of goyaml, but
> also of the stock encoding/json package, and likely many other
> marshaling packages out there. There's already partial agreement from
> the core team that this is not okay and must be addressed before Go 1.3.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1275410
>
> Title:
> tests fail with gccgo
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/goyaml/+bug/1275410/+subscriptions
>

Revision history for this message
Tim Penhey (thumper) wrote :

Tests still fail with gccgo 4.9 today on trusty.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

The "fix" for the problem, which made gc behave as gccgo, indeed broke
a lot of people and got reverted for 1.3. The right change would be to
fix gccgo at this point so that it works as gc, until upstream decides
how to fix it properly. It won't match the behavior of gccgo today.

On Wed, May 14, 2014 at 12:38 AM, Tim Penhey <email address hidden> wrote:
> Tests still fail with gccgo 4.9 today on trusty.
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1275410
>
> Title:
> tests fail with gccgo
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/goyaml/+bug/1275410/+subscriptions

--

gustavo @ http://niemeyer.net

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.