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

HTTPS support, global header injection, improved reporter output #5

Merged
merged 10 commits into from Oct 24, 2013

Conversation

ecordell
Copy link
Contributor

This pull request does a few things:

  • Adds support for https (requests use the https module if the server url begins with https)
  • Allows injecting global headers through dredd configuration, like so:
    dredd = new Dredd {
      blueprintPath : './blueprint.md'
      server: 'https://foo.bar/'
      request:
        headers:
          'X-Api-Key' : '111-222-333-444-555-666'
      options:
        reporter: 'junit'
        debug: false
    }
  • Improves error messages
    • Example cli output:
    fail: GET /foo/bar/?aq=test
    fail: headers: Value of the ‘content-type’ must be application/json.
    body: Real and expected data does not match.

    request: 
    {
        "host": "foo.bar",
        "port": null,
        "path": "/foo/bar/?aq=test",
        "method": "GET",
        "headers": {
            "Content-Type": "application/json",
            "User-Agent": "Dredd/0.1.3 (Darwin 12.4.0; x64)",
            "X-Api-Key": "111-222-333-444-555-666"
        }
    }

    expected: 
    {
        "headers": {
            "Content-Type": "application/json"
        },
        "body": "",
        "statusCode": "200"
    }

    actual: 
    {
        "headers": {
            "content-type": "text/html",
            "date": "Wed, 23 Oct 2013 22:30:48 GMT",
            "server": "nginx/1.4.1",
            "vary": "Cookie",
            "content-length": "7486",
            "connection": "keep-alive"
        },
        "body": "error",
        "status": 404
    }
  • XUnit output much better as well.

This might be enough to support basic auth (#3), though I haven't tried it, and you'd need to construct the Authorization header yourself and pass it in (username:password encoded in base64). I only needed https and custom headers, so that's all I added.

I'm using bamboo's Junit Parser as the template for the junit reporter's output, and have made some changes to improve that as well.

Coverage went down just a bit :/ but it's still pretty good!

I plan to remove the cli library from everything but bin/dredd and use another logger internally. Using cli everywhere has made it so that passing in silent or debug flags when consuming dredd in another app doesn't work. Plus using something like winston means you could have custom log levels. That will be a separate pull request though!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling cf17fce on localmed:allow-auth-headers into e22c4b0 on apiaryio:master.

@ecordell
Copy link
Contributor Author

I had to add /lib to the project so that it could actually be used in other projects. Am I missing something? Should I revert those commits?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1bcaade on localmed:allow-auth-headers into e22c4b0 on apiaryio:master.

@netmilk
Copy link
Contributor

netmilk commented Oct 24, 2013

Hi Evan,

/lib folder should not be versioned and definitely should not be part of the GitHub repository. /lib directory is distributed as a part of the NPM package, it's built during the prepublish hook and the main section in the package.json is pointing to the compiled pure JavaScript. This mean that coffee-script is not a runtime dependency at all and Dredd can be used programatically in other software as I mentioned here.

Please revert all commits regarding /lib dir, I'll merge this PR afterwards and I'll publish the new version to the NPM repository.

Thanks again for the bunch of awesome work!

All the best
Adam

@netmilk netmilk closed this Oct 24, 2013
@netmilk netmilk reopened this Oct 24, 2013
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1bcaade on localmed:allow-auth-headers into e22c4b0 on apiaryio:master.

@netmilk
Copy link
Contributor

netmilk commented Oct 24, 2013

Just one note about the headers inkjection. Headers and parameters injection (top to bottom inheritance) should be part of the API Blueprint format. You will be able to define some global headers and parameters in the API Name & Overview Section like tokens, paging or sorting.

For this moment I see headers injection on the Dredd level as very useful. Especially for purpose of HTTP basic authentication.

A

…compiling" and "Adds /lib"

This reverts commit e3d0280.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 50815cc on localmed:allow-auth-headers into e22c4b0 on apiaryio:master.

@ecordell
Copy link
Contributor Author

Reverted the /lib stuff. I was trying to point npm to a branch on my fork of dredd so that I could test it in production. For whatever reason that doesn't actually build the library, so I mistakenly thought to add the compiled source.

As for the headers: I saw that the spec supports inheriting headers, but that doesn't help when you need to generate the header dynamically (for sessions or auth or something else).

Thanks!

@netmilk
Copy link
Contributor

netmilk commented Oct 24, 2013

Evan,
/lib dir comes from the distributed npm package from the NPM registry only when you install it wit npm install dredd If you want to use dredd package code without installing from it the NPM registry in your local environment just run the script/build command manually.
Obviusely, you can use some more intelligent solution e.g grunt watch which will watch for changes is /src dir and it will rebuild the /lib on every change to prevent
Adam

netmilk pushed a commit that referenced this pull request Oct 24, 2013
HTTPS support, global header injection, improved reporter output
@netmilk netmilk merged commit 2dd032b into apiaryio:master Oct 24, 2013
@ecordell ecordell deleted the allow-auth-headers branch January 21, 2014 14:32
honzajavorek pushed a commit that referenced this pull request Jan 27, 2016
honzajavorek pushed a commit that referenced this pull request Feb 2, 2016
Added .gitattributes so users of the package don't have unnecessary a…
honzajavorek pushed a commit that referenced this pull request Jul 18, 2016
artem-zakharchenko pushed a commit that referenced this pull request Oct 9, 2019
artem-zakharchenko pushed a commit that referenced this pull request Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants