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

failing unit test #9

Closed
wants to merge 1 commit into from
Closed

failing unit test #9

wants to merge 1 commit into from

Conversation

mikedanese
Copy link

Do not merge, failing test. I was trying to write some tests for embedded structs as there weren't any and I'm getting some unexpected behavior on the last test case:

{
    src: embeddingStruct{
        embeddedStruct{"foo"},
    },
    dst: embeddingStruct{
        embeddedStruct{"bar"},
    },
    expected: embeddingStruct{
        embeddedStruct{"bar"},
    },
},

which is failing with this output:

=== RUN TestEmbeddedStruct
--- FAIL: TestEmbeddedStruct (0.00s)
    mergo_test.go:132: unexpected output
        expected:
        {embeddedStruct:{A:bar}}
        saw:
        {embeddedStruct:{A:foo}}
FAIL
exit status 1
FAIL    github.com/imdario/mergo    0.004s

I would expect dst to keep it's "bar" field as it's not the default value of string. Per the Merge documentation, Merge sets fields' values in dst from src if they have a zero value of their type. Is this unexpected behavior?

@darccio
Copy link
Owner

darccio commented Feb 1, 2015

Yes, it looks like unexpected behavior.

@darccio
Copy link
Owner

darccio commented Apr 6, 2015

Ok, I checked this and now I understand what you meant.

Mergo doesn't override values unless you use the new methods from #8: MergeWithOverwrite and MapWithOverwrite.

Mergo only assigns values when destination has a zero value, so those functions are the way to go if you want overwriting behavior.

Happy merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants