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

Add missing deserialization tests #141

Open
timoreimann opened this issue Apr 15, 2016 · 3 comments
Open

Add missing deserialization tests #141

timoreimann opened this issue Apr 15, 2016 · 3 comments

Comments

@timoreimann
Copy link
Collaborator

There's a fairly large discrepancy between the number of fields we populated the test YML file with and the amount of test checks we have. Effectively, this boils down to a lack of test coverage with regards to deserialization. At the same time, we have lots of duplication in the test messages. While this can be reasonable at times, in our case it mostly seems superfluous.

We should attempt to close the coverage gap and simultaneously reduce the amount of spec duplication. Hopefully this can be accomplished by distinguishing better between YML content that pertains to deserialization testing and other that affects code logic testing.

Discovered along #140.

@mhausenblas
Copy link

Unsure if this is the best place or if you want me to open a new issue for this. When I unmarshal:

"docker": {
    "image": "python:3",
    "network": "BRIDGE",
    "portMappings": [
        {
            "containerPort": 8080,
            "hostPort": 0
        }
    ]
}

then I get a:

FATA[0000] Failed to create application null. Error: Marathon API error: Object is not valid (attribute '': )` 

I need to add: "servicePort": 0, "protocol": "tcp" there to make it work. This shouldn't be necessary since the latter two attributes are not required.

@timoreimann
Copy link
Collaborator Author

timoreimann commented Apr 28, 2016

@mhausenblas In which context does the error show up exactly? You say unmarshal but the error message seems to indicate that this is happening when you create an application (i.e., send a POST to the Marathon API). If so, how did you initialize your Application instance?

Also, what version of Marathon are you testing against?

I think we should address this problem in a new issue. While you could be hitting a case that proper test coverage (the goal of this ticket) may have prevented, the immediate fix should probably be handled separately (and more quickly).
Could you please create another ticket? Thanks!

@mhausenblas
Copy link

Thanks @timoreimann and done: #144

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

No branches or pull requests

2 participants