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

trailing newline fails text/plain #67

Open
jsongHBO opened this issue May 5, 2014 · 8 comments
Open

trailing newline fails text/plain #67

jsongHBO opened this issue May 5, 2014 · 8 comments

Comments

@jsongHBO
Copy link

jsongHBO commented May 5, 2014

Is there a solution for
apiaryio/snowcrash#56
?

due to the inserted \n, comparing text/plain will always fail the dredd test.

Here's my transaction in the after hook

{ name: 'GET animals > Via id > Retuning text',
  id: 'GET /animals/G1234',
  host: 'localhost',
  port: '3000',
  request: 
   { body: '{}\n',
     headers: 
      { Accept: 'text/plain',
        'content-type': 'text/plain',
        'User-Agent': 'Dredd/0.3.1 (Darwin 13.1.0; x64)',
        'Content-Length': 3 },
     uri: '/animals/G1234',
     method: 'GET' },
  expected: 
   { headers: { 'Content-Type': 'text/plain' },
   **body: 'G1234\n',**
     statusCode: '200' },
  fullPath: '/animals/G1234',
  real: 
   { statusCode: 200,
     headers: 
      {
        'content-type': 'text/plain',
        'content-length': '5',
        connection: 'keep-alive' },
     **body: 'G1234'** } }

and my markdown is simply

    + Response 200 (text/plain)

        ```
        G1234
        ```
@netmilk
Copy link
Contributor

netmilk commented Jul 1, 2014

Thank you for the report, but this is still a Snowcrash issue as you mentioned.

@JanBednarik
Copy link

Because it can't be solved in snowcrash, what do you think about command line option for dredd --ignore-newline which will ignore trailing newline in body?

@honzajavorek
Copy link
Contributor

Related to #343.

@honzajavorek
Copy link
Contributor

This is still an issue. Workaround can be done by hooks.

@honzajavorek
Copy link
Contributor

This is specific to API Blueprint and quite dependent on how Drafter (the API Blueprint parser) handles the situation. The nature of the format makes it difficult for the parser to decide whether the body payload ends with a newline or not. Hence the parser always adds the newline. In JSON, this is not a problem, but in plain text formats, this is a serious issue.

Dredd could implement a strategy around this, but I'm not sure what would be the best way to approach this. In most cases, the newline is unwanted, but what if user expects the newline? Dredd cannot just always remove the newline.

@honzajavorek
Copy link
Contributor

My proposal for solution would be to implement following algorithm. If the list items below are all true:

...then assume those response bodies are equal. As this is specific to one API description format, it should be implemented in Dredd, not in Gavel.js.

@artem-zakharchenko
Copy link
Contributor

@honzajavorek is this a general expectations toward gavel to ignore newlines? If so, I suggest it solved on the Gavel's side.

If it's a workaround specific for APIB, then it diverges the behavior between the default behavior (gavel validating body as-is), and Dredd swallowing newlines for APIB only. If the newlines are produced by mistake by parser, it's a parser issue as you've already referred to:

In JSON, this is not a problem, but in plain text formats, this is a serious issue.

Can we get some more insights why is this serious and cannot be tackled on the parser's side?

@honzajavorek
Copy link
Contributor

honzajavorek commented Oct 23, 2019

I'd suggest sth like (pseudocode):

result = gavel.validate({ body: "ahoj\n" }, { body: "ahoj" })

if (result.kind == text && mediaType == apib && !result.fields.body.valid && expected.body.rtrim('\n') == actual.body) {
    result = setBodyAsValid(result)
}

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

No branches or pull requests

5 participants