portman icon indicating copy to clipboard operation
portman copied to clipboard

multiple content types in openAPI spec result in SyntaxError in contract tests

Open scags9876 opened this issue 3 years ago • 5 comments

portman version 1.3.2

Using the following openAPI spec document: https://gist.github.com/scags9876/889d739a72217f68ef3d1d93b2a6a034

When contract tests are generated, there is a SyntaxError:

 1.  SyntaxError                      Identifier 'schema' has already been declared                                                                                   
                                      at test-script                                                                                                                  
                                      inside "Get Pastry by name"  

Looking at the generated tests in the collection, I see:

// Response Validation
const schema = {"title":"Root Type for Pastry","description":"The root of the Pastry type's schema.","type":"object","properties":{"name":{"description":"Name of this pastry","type":"string"},"description":{"description":"A short description of this pastry","type":"string"},"size":{"description":"Size of pastry (S, M, L)","type":"string"},"price":{"description":"Price (in USD) of this pastry","type":"number"},"status":{"description":"Status in stock (available, out_of_stock)","type":"string"}},"example":{"name":"My Pastry","description":"A short description os my pastry","size":"M","price":4.5,"status":"available"}}

// Validate if response matches JSON schema 
pm.test("[GET]::/pastry/:name - Schema is valid", function() {
    pm.response.to.have.jsonSchema(schema,{unknownFormats: ["int32", "int64"]});
});

// Response Validation
const schema = {"title":"Root Type for Pastry","description":"The root of the Pastry type's schema.","type":"object","properties":{"name":{"description":"Name of this pastry","type":"string"},"description":{"description":"A short description of this pastry","type":"string"},"size":{"description":"Size of pastry (S, M, L)","type":"string"},"price":{"description":"Price (in USD) of this pastry","type":"number"},"status":{"description":"Status in stock (available, out_of_stock)","type":"string"}},"example":{"name":"My Pastry","description":"A short description os my pastry","size":"M","price":4.5,"status":"available"}}

// Validate if response matches JSON schema 
pm.test("[GET]::/pastry/:name - Schema is valid", function() {
    pm.response.to.have.jsonSchema(schema,{unknownFormats: ["int32", "int64"]});
});

It looks like, since there are multiple content types in the responses.status.content map, as the spec allows, a test is added for each content type. Perhaps a separate request should be added to the collection for each content type with the content type header specified, and the schema validation for only that content type should be run in the tests for that request. Or, explicitly state that portman only supports json responses and do not generate tests for other content types.

Additionally to the issue above, the collection tests also include:

// Validate if response header has matching content-type
pm.test("[GET]::/pastry/:name - Content-Type is application/json", function () {
   pm.expect(pm.response.headers.get("Content-Type")).to.include("application/json");
});

// Validate if response header has matching content-type
pm.test("[GET]::/pastry/:name - Content-Type is text/xml", function () {
   pm.expect(pm.response.headers.get("Content-Type")).to.include("text/xml");
});

One of which will likely fail. The same solutions I describe above could fix this too, or I'm sure some other method is possible.

Further info. using the cli-options:

{
  "url": "https://gist.githubusercontent.com/scags9876/889d739a72217f68ef3d1d93b2a6a034/raw/d979161b9f468c90c6203035c0de72dec7df3622/APIPastry-openapi-fixed-price.yaml",
  "baseUrl": "http://0.0.0.0:4010",
  "output": "collection.postman.json",
  "includeTests": true,
  "syncPostman": false,
  "runNewman": true
}

Let me know if I can provide further info.

scags9876 avatar Jul 14 '21 04:07 scags9876

Thanks @scags9876 for bringing it to our attention.

There are a number of ways to solve this, so we are looking into an option that is easy to configure with the test injection.

The different content-types can be considered response variations (tackled in the variation testing part) but they also are part of the contract testing.

You already hinted at a number of options to solve this, which we will take into account.

While we work on enhancing the support for multiple content-types, we will introduce a temporary fix, where we will take the 1st content-type that is defined in the OpenAPI response.

thim81 avatar Jul 14 '21 09:07 thim81

@scags9876 in the 1.3.3 release we already applied the temporary fix, to provide working tests with multiple content-types. We will continue with the enhancement of supporting multiple content-types. This will take more time finalise.

thim81 avatar Jul 14 '21 12:07 thim81

Thanks for the quick response! Good solution on taking the first content-type for now. Agreed that a more full solution will take time to get it right.

scags9876 avatar Jul 14 '21 15:07 scags9876

@scags9876 We are actively working on a configuration option for handling the multiple content-types. I m wondering to hear your feedback. The goal is to make the test suite configuration easy but still allow flexibility.

Idea: openapi allows to define multiple responses and multiple content-types, Portman would provide a "standard" that picks 1 response and 1 content-type of that response, meaning a 2xx response and 1 content-type for the 2xx response.

Some questions:

  • Would you be ok with this idea? Or have different expectations?

  • would you be ok to separate the contract testing for the multiple content-types in 2 parts: one for standard response: 2xx - 1 content-type and have the other content types split off in a variation test? Which would still lead to different postman request per content type, but the variations would be grouped in the variation test folder. Keeping the main request cleaner.

thim81 avatar Aug 23 '21 13:08 thim81

That seems like an acceptable solution. I have not played with the variation tests very much, so I'm not sure if it would fit my use case exactly.

My ideal solution would be to have a all content types be exercised in the contract tests: one request in the collection for each content type. That is the way I've hand written contract tests in postman in the past. That is, each path gets 2 requests in the collection: one for json, one for xml.

If it's your preference to keep them in the variation tests and that flows better for you, then I'm fine with adapting my testing strategy if this tool fits my needs :)

scags9876 avatar Aug 24 '21 23:08 scags9876