matryer / silk

Markdown based document-driven RESTful API testing.
http://silktest.org/
GNU General Public License v2.0
942 stars 69 forks source link

Problem with expected JSON #39

Closed ivancevich closed 8 years ago

ivancevich commented 8 years ago

Hi, First of all I'd like to thank you for such a useful library. It's both simple to use and pretty powerful. I want to report an issue I've been facing using Silk. I've discovered it adding a property to the JSON I'm expecting in the response. Here is the JSON which works:

{
    "is_complete": true,
    "user": {
        "id": 1,
        "name": "Juan Carlos",
        "email": "jc@foo.com",
        "phone": "541122334455",
        "gender": "male",
        "gender_of_interest": "women",
        "birthday": "1990-02-15T00:00:00Z",
        "height": 170,
        "job_title": "Software Engineer",
        "company": "Whim",
        "school": "UADE",
        "degree": {
            "id": 3,
            "description": "Bachelor's"
        },
        "descriptors": [
            {
                "id": 5,
                "description": "Sarcastic"
            },
            {
                "id": 10,
                "description": "Night owl"
            }
        ],
        "created_at": "2016-02-26T14:00:00Z",
        "updated_at": "2016-03-16T12:00:00Z"
    }
}

And here is the JSON which breaks my tests (notice the ethnicities array was added):

{
    "is_complete": true,
    "user": {
        "id": 1,
        "name": "Juan Carlos",
        "email": "jc@foo.com",
        "phone": "541122334455",
        "gender": "male",
        "gender_of_interest": "women",
        "birthday": "1990-02-15T00:00:00Z",
        "height": 170,
        "job_title": "Software Engineer",
        "company": "Whim",
        "school": "UADE",
        "degree": {
            "id": 3,
            "description": "Bachelor's"
        },
        "descriptors": [
            {
                "id": 5,
                "description": "Sarcastic"
            },
            {
                "id": 10,
                "description": "Night owl"
            }
        ],
        "ethnicities": [
            {
                "id": 1,
                "description": "Asian"
            },
            {
                "id": 3,
                "description": "Latino"
            }
        ],
        "created_at": "2016-02-26T14:00:00Z",
        "updated_at": "2016-03-16T12:00:00Z"
    }
}

The error that I get is the following:

[GIN] 2016/05/04 - 15:46:38 | 404 |       2.599µs | 127.0.0.1 |   iJS     /
body expected:  

{
    "is_complete": true,
    "user": {
        "id": 1,
        "name": "Juan Carlos",
        "email": "jc@foo.com",
        "phone": "541122334455",
        "gender": "male",
        "gender_of_interest": "women",
        "birthday": "1990-02-15T00:00:00Z",
        "height": 170,
        "job_title": "Software Engineer",
        "company": "Whim",
        "school": "UADE",
        "degree": {
            "id": 3,
            "description": "Bachelor's"
        },
        "descriptors": [
            {
                "id": 5,
                "description": "Sarcastic"
            },
            {
                "id": 10,
                "description": "Night owl"
            }
        ],
        "ethnicities": [
            {
                "id": 1,
                "description": "Asian"
            },
            {
                "id": 3,
                "description": "Latino"
            }
        ],
        "created_at": "2016-02-26T14:00:00Z",
        "updated_at": "2016-03-16T12:00:00Z"
    }
}  

actual:  

404 page not found  

--- FAIL: iJS zUxMiIsInR5c 
 users.md:22 - body doesn't match  
--- FAIL: TestUsers (0.00s)
FAIL
coverage: 0.0% of statements

Thanks in advance. JC

matryer commented 8 years ago

It looks like a simple 404 error?!

Mat

On 4 May 2016, at 17:17, JC Ivancevich notifications@github.com wrote:

Hi, First of all I'd like to thank you for such a useful library. It's both simple to use and pretty powerful. I want to report an issue I've been facing using Silk. I've discovered it adding a property to the JSON I'm expecting in the response. Here is the JSON which works:

{ "is_complete": true, "user": { "id": 1, "name": "Juan Carlos", "email": "jc@foo.com", "phone": "541122334455", "gender": "male", "gender_of_interest": "women", "birthday": "1990-02-15T00:00:00Z", "height": 170, "job_title": "Software Engineer", "company": "Whim", "school": "UADE", "degree": { "id": 3, "description": "Bachelor's" }, "descriptors": [ { "id": 5, "description": "Sarcastic" }, { "id": 10, "description": "Night owl" } ], "created_at": "2016-02-26T14:00:00Z", "updated_at": "2016-03-16T12:00:00Z" } } And here is the JSON which breaks my tests (notice the ethnicities array was added):

{ "is_complete": true, "user": { "id": 1, "name": "Juan Carlos", "email": "jc@foo.com", "phone": "541122334455", "gender": "male", "gender_of_interest": "women", "birthday": "1990-02-15T00:00:00Z", "height": 170, "job_title": "Software Engineer", "company": "Whim", "school": "UADE", "degree": { "id": 3, "description": "Bachelor's" }, "descriptors": [ { "id": 5, "description": "Sarcastic" }, { "id": 10, "description": "Night owl" } ], "ethnicities": [ { "id": 1, "description": "Asian" }, { "id": 3, "description": "Latino" } ], "created_at": "2016-02-26T14:00:00Z", "updated_at": "2016-03-16T12:00:00Z" } } The error that I get is the following:

[GIN] 2016/05/04 - 15:46:38 | 404 | 2.599µs | 127.0.0.1 | iJS / body expected:

{ "is_complete": true, "user": { "id": 1, "name": "Juan Carlos", "email": "jc@whim.com", "phone": "541140492533", "gender": "male", "gender_of_interest": "women", "birthday": "1990-02-15T00:00:00Z", "height": 170, "job_title": "Software Engineer", "company": "Whim", "school": "UADE", "degree": { "id": 3, "description": "Bachelor's" }, "descriptors": [ { "id": 5, "description": "Sarcastic" }, { "id": 10, "description": "Night owl" } ], "ethnicities": [ { "id": 1, "description": "Asian" }, { "id": 3, "description": "Latino" } ], "created_at": "2016-02-26T14:00:00Z", "updated_at": "2016-03-16T12:00:00Z" } }

actual:

404 page not found

--- FAIL: iJS zUxMiIsInR5c users.md:22 - body doesn't match
--- FAIL: TestUsers (0.00s) FAIL coverage: 0.0% of statements Thanks in advance. JC

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub

ivancevich commented 8 years ago

Yeah, I thought the same. But it's not actually triggered by the API. And if I remove that new property ethnicities it just works. I think there should be something else. What's that iJS zUxMiIsInR5c mean?

Thanks JC

matryer commented 8 years ago

Yeah that looks weird. Can you share real code somehow?

On 4 May 2016, at 17:43, JC Ivancevich notifications@github.com wrote:

Yeah, I thought the same. But it's not actually trigger by the API. And if I remove that new property ethnicities it just works. I think there should be something else. What's this iJS zUxMiIsInR5c mean?

Thanks JC

— You are receiving this because you commented. Reply to this email directly or view it on GitHub

ivancevich commented 8 years ago

I've created this gist including the markdown docs and the test file: https://gist.github.com/ivancevich/4c8802363b35d56e299da9a25a8dba7d

It's mostly mocked, except from the API itself.

Thanks JC

ivancevich commented 8 years ago

Let me know if you need anything else. I'm using Gin as the web framework by the way.

antekresic commented 8 years ago

I believe this is connected to using scanner.Bytes() as a ParseLine argument here: https://github.com/matryer/silk/blob/master/parse/parser.go#L87

Line.Bytes and Line.Comment are pointing to same array and it gets overwritten.

The underlying array may point to data that will be overwritten by a subsequent call to Scan. https://golang.org/pkg/bufio/#Scanner.Bytes

Fix would be to explicitly copy the bytes to a new array.

Let me know if you need a PR :)

ivancevich commented 8 years ago

Hi @antekresic I'm not sure I understood you well.

Copying the bytes got from scanner.Bytes() makes all tests to always pass, even when they should fail. I've done something like:

bytes := scanner.Bytes()
text := make([]byte, len(bytes))
copy(text, bytes)
line, err := ParseLine(n, text)

Did I make something wrong? Thanks JC

matryer commented 8 years ago

Can we get a unit test to prove this bug? Then we can get this fix in.

Thanks for investigating - strange one.

Mat

On 13 May 2016, at 22:03, JC Ivancevich notifications@github.com wrote:

Hey @antekresic I copied the bytes as you suggested and it's working:

var bytes []byte copy(bytes, scanner.Bytes()) line, err := ParseLine(n, bytes) Thanks a lot JC

— You are receiving this because you commented. Reply to this email directly or view it on GitHub

antekresic commented 8 years ago

Here is the unit test proving the bug: https://gist.github.com/antekresic/0d72df75ef7991dcff8c89877113b182

@ivancevich fix looks good, tests are all passing since this behavior hasn't been tested before.

matryer commented 8 years ago

Is this solved in https://github.com/matryer/silk/pull/40?

antekresic commented 8 years ago

I believe so, if @ivancevich confirms.

ivancevich commented 8 years ago

I'll test it again in a couple of minutes. I let you know then.

ivancevich commented 8 years ago

Yes, it worked just fine with the fixes in #40 😄 Thanks a lot @antekresic and @matryer !