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

Can't Dredd set URI parameters default value? #709

Open
motoyasu-saburi opened this issue Feb 6, 2017 · 7 comments
Open

Can't Dredd set URI parameters default value? #709

motoyasu-saburi opened this issue Feb 6, 2017 · 7 comments

Comments

@motoyasu-saburi
Copy link

I try set default value for URI parameters at this issue comment.
#541 (comment)

but, Dredd display "warning" or set mistake default value at URI parameter.

node ver : v6.9.1
dredd ver: v2.2.5
dredd --version output: 3.10.8

pattern 1

# Group ApplicationController

## applications [/api/applications{?offset,limit,sortOrder,text,unreadOnly}]

### applications list [GET]

+ Parameters
  + offset (number, optional)
      - Default: `0`
  + limit (number, optional)
      - Default: `20`
  + sortOrder: `DESC` (optional, string)
  + text (optional, string)
  + unreadOnly (boolean, optional)
      - Default: `false`

+ Response 200

...

Result pattern 1 :

fail: GET /api/applications?offset=0&limit=20&sortOrder=DESC&text=20&unreadOnly=false duration: 43ms

non errors and warnings.
but, Dredd set mistake value 'text=20' for URI Parameter.


pattern 2

# Group ApplicationController

## applications [/api/applications{?offset,limit,sortOrder,text,unreadOnly}]

### applications list [GET]

+ Parameters
  + offset: `0` (number, default)
  + limit: `20` (number, default)
  + sortOrder: `DESC` (optional, string)
  + text (optional, string)
  + unreadOnly: `false` (boolean, default)

+ Response 200
Result of pattern 2 :

warn: Parser warning in file 'document.md': (warning code 3) unable to parse additional parameter traits, expected '([required | optional], [<type> | enum[<type>])', e.g. '(optional, string)' on line 32
warn: Parser warning in file 'document.md': (warning code 3) unable to parse additional parameter traits, expected '([required | optional], [<type> | enum[<type>])', e.g. '(optional, string)' on line 33
warn: Parser warning in file 'document.md': (warning code 3) unable to parse additional parameter traits, expected '([required | optional], [<type> | enum[<type>])', e.g. '(optional, string)' on line 36

fail: GET /api/applications?offset=0&limit=20&sortOrder=DESC&unreadOnly=false duration: 51ms

correct value.
but, display warn.

@honzajavorek
Copy link
Contributor

honzajavorek commented Feb 6, 2017

@motoyasu-saburi I'll take a deeper look later, but at first sight, I'd say there might be a problem with using 2 spaces for indentation of the + Parameters list items. I think API Blueprint parser can correctly handle only 4 spaces indentation.

@honzajavorek
Copy link
Contributor

honzajavorek commented Feb 6, 2017

Moreover, it looks like you even use mixed indentation:

␣␣+ offset (number, optional)
␣␣␣␣␣␣- Default: `0`

(by two, then by two + four)

@motoyasu-saburi
Copy link
Author

@honzajavorek Thank you quickly answer!
I tried 4 spaces indentation pattern.
however, that won't change anything 😭

4 spaces pattern.

FORMAT: 1A
HOST: http://localhost:9000

# hoge (foo)

# Group ApplicationController

## application

## applications [/api/applications{?offset,limit,sortOrder,text,unreadOnly}]

### application list [GET]

get applications list

+ Parameters
    + offset (number, optional)
        - Default: `0`
    + limit (number, optional)
        - Default: `20`
    + sortOrder: `DESC` (optional, string)
    + text (optional, string)
    + unreadOnly (boolean, optional)
        - Default: `false`

+ Response 200

+ Response 400

+ Response 401

result

info: Configuration './dredd.yml' found, ignoring other arguments.
info: Beginning Dredd testing...
info: Found Hookfiles: 0=./hook.js
fail: GET /api/applications?offset=0&limit=20&sortOrder=DESC&text=20&unreadOnly=false duration: 157ms
info: Displaying failed tests...
...

Dredd set mistake URI parameter test=20.
hook.js and dredd.yml is non custom.

@honzajavorek
Copy link
Contributor

I went to Apiary and tried to render documentation from your API Blueprint. It looks like the markup is definitely correct, because the documentation renders correctly:

image

When I try to run the latest Dredd, I was able to reproduce your problems:

$ dredd apiary.apib http://example.com
info: Beginning Dredd testing...
fail: GET /api/applications?offset=0&limit=20&sortOrder=DESC&text=20&unreadOnly=false duration: 314ms
info: Displaying failed tests...
fail: GET /api/applications?offset=0&limit=20&sortOrder=DESC&text=20&unreadOnly=false duration: 314ms
...

This is a bug in Dredd, or more precisely, in Dredd Transactions. Thanks for reporting this!

If you want to work on a fix, I'd expect compileParameters() to be the culprit. If you don't feel like contributing a solution, you could at least send a failing test. What do you think?

@honzajavorek
Copy link
Contributor

I suspect there would be a variable in a loop iterating over parameters, which doesn't get cleared out between iterations, thus text, while it should stay empty, erroneously inherits the value specified for limit. But I didn't look into the code, that's just how I imagine the problem could happen.

@motoyasu-saburi
Copy link
Author

I thought this cause is here 🤔

defaultValue is not initialized in loop process.
https://github.com/apiaryio/dredd-transactions/blob/master/src/compile.coffee#L166

If initialization is not done, I think that the previous defaultValue value will be inherited to the next defaultValue.

@honzajavorek
Copy link
Contributor

@motoyasu-saburi I think that's it! Would you send a PR to fix this? I'd be happy to provide help on how to approach this and how to write a test.

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

2 participants