apiaryio / dredd

Language-agnostic HTTP API Testing Tool
https://dredd.org
MIT License
4.18k stars 279 forks source link

Dredd unable to correctly handle blueprint with multiple requests #78

Open honzajavorek opened 10 years ago

honzajavorek commented 10 years ago

Example blueprint: http://docs.apiblueprintapi.apiary.io/ Possibly related to #25.

netmilk commented 10 years ago

Thanks for the report! Can you provide here raw blueprint for replicating the issue?

honzajavorek commented 10 years ago

Sure, here: https://github.com/apiaryio/apiblueprintorg/blob/master/apiary.apib

Compose [POST] (line 214) has two possible inputs (two requests), but only one output (one response).

netmilk commented 9 years ago

Ok, this is verified. But what's the proposed behaviour? Should Dredd test the response for each request?

$ curl -s https://raw.githubusercontent.com/apiaryio/apiblueprintorg/master/apiary.apib > multi-blueprint.md $ dredd multi-blueprint.md http://localhost --names warn: Runtime compilation warning: Multiple requests in example, using first. on Blueprint Manipulation > Blueprint Composer > Compose info: Beginning Dredd testing... info: API > Service Root > List info: Blueprint Manipulation > Blueprint Parser > Parse > Example 1 info: Blueprint Manipulation > Blueprint Parser > Parse > Example 2 info: Blueprint Manipulation > Blueprint Composer > Compose

honzajavorek commented 9 years ago

As there can be multiple requests as well as multiple responses, I think the algorithm would be following (high-level coffee-like pseudocode):

for example in examples
  for request in example.requests
    for response in example.responses
      test(request, response)

That's probably the best dredd can do.

netmilk commented 9 years ago

Maybe I'm not right, but I don't think so. This code will mean that in the example if there is no request but only responses, no test is created and also if there is no response only request no test is created either.

honzajavorek commented 9 years ago

Well, yes, good catch. I did not think about these cases while writing the code. Depends on what is the intended behavior, what the blueprint author expects in such situation - one option is to skip cases where the pair is not complete (code above) or to provide a default request and default response:

for example in examples
  for request in (example.requests or [defaultRequest])
    for response in (example.responses or [defaultResponse])
      test(request, response)

Right now I don't know what's Apiary app's behavior in these cases and if there's a concept of default request/response already. I'd need to experiment with it for a while.

obihann commented 9 years ago

poke This issue is coming up for me now, any progress?

w-vi commented 9 years ago

Hi @obihann this hasn't been addressed yet, next week we'll be able to give you some ETA.

obihann commented 9 years ago

@w-vi If this is a manpower issue let me know I can likely help out. Thanks for the reply though.

w-vi commented 9 years ago

@obihann It is a manpower problem at least to some degree but first we have to decide how to handle the edge cases (missing request/response etc.), I guess we will discuss it once @netmilk is back in game.

obihann commented 9 years ago

Just a poke to bring this back on the table. @w-vi @netmilk

netmilk commented 9 years ago

Hey @obihann,

thank you very much for offering help! It's awesome! Would you be so kind and share a blueprint example with multiple request and responses and proposed test matrix for these examples and request -response pairs?

obihann commented 9 years ago

No problem, I'll have that for you later today.

obihann commented 9 years ago

### Delete a Object [DELETE /{id}{?permanent}]

+ Parameters

    + permanent: `0` (number, optional) - Set if the object will be permanently deleted
        + Members
            + `0` - Flag the object as deleted however do not remove it from our system
            + `1` - Permanently delete this object

+ Request

+ Response 202

So this is an example of some markdown I have, it represents a single API endpoint that accepts an optional URL param that can be either 0 or 1. Ideally I would like dredd to acknowledge this and execute the request for each of the possible URL param values.

An alternative solution that may required updates to the MSON spec would be a way to outline what URL param values are includes in each request.

obihann commented 9 years ago

Another use case would be two request's one with a body one without, the one without would return something like a 400, perhaps with a explicit error. The request with a body would return something like a 200. The point here being to test that invalid requests do not break (500) the server and are handled properly.

honzajavorek commented 9 years ago

This is biting me again as well as #25. The use case is pretty much the same as before - I am working on similar parsing service as in my previous comments. It uses multiple requests and responses (M:N relation) heavily. I can not use Dredd for testing such API as it always takes just the first pair.

pksunkara commented 7 years ago

Since we have moved to API Description Refract, we can support this in a backward compatible way.

Assume, we have request response pairs (RRP from now) in an API Blueprint as shown below:

+ Request A
+ Request B
+ Response 200
+ Response 400
+ Request C
+ Response 403

API Description refract generates 5 RRP. They are as follows:

RRP 1
====
+ Request A
+ Response 200

RRP 2
====
+ Request B
+ Response 200

RRP 3
====
+ Request A
+ Response 400

RRP 4
=====
+ Request B
+ Response 400

RRP 5
====
+ Request C
+ Response 403

Actual Behaviour

Dredd currently only tests RRP 1 and RRP 5 as the first pair and the second pair.

Expected Behaviour

Dredd needs to test all 5 pairs. But to ensure the backward compatibility of the order of the pairs that are being tested, we will add the newer pairs to the end. Which means dredd will test them in the following order: RRP 1, RRP 5, RRP 2, RRP 3, RRP 4

honzajavorek commented 7 years ago

@pksunkara As we discussed previously, I think adding new transactions is still a breaking change, but that could be solved. We could either make this breaking change, or to skip the transactions by default and allow people to unskip them in hooks.

And when I was thinking about writing you the sentence above, I just realized a problem with this. What would be transaction names of the added requests? There is no way how to distinguish such transactions in current transaction names. According to how it works now, the transaction names would be:

A-200 ... Lorem > Ipsum > Lorem Ipsum > Example 1
A-400 ... Lorem > Ipsum > Lorem Ipsum > Example 1
B-200 ... Lorem > Ipsum > Lorem Ipsum > Example 1
B-400 ... Lorem > Ipsum > Lorem Ipsum > Example 1
C-403 ... Lorem > Ipsum > Lorem Ipsum > Example 2

That's it. They wouldn't be addressable. So I'm really afraid this will be possible only after #227 are done 😞

pksunkara commented 7 years ago

The transaction names would be:

A-200 ... Lorem > Ipsum > Lorem Ipsum > Example 1
A-400 ... Lorem > Ipsum > Lorem Ipsum > Example 3
B-200 ... Lorem > Ipsum > Lorem Ipsum > Example 4
B-400 ... Lorem > Ipsum > Lorem Ipsum > Example 5
C-403 ... Lorem > Ipsum > Lorem Ipsum > Example 2

I thought that part was clear from my comment.