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

Routes that redirect result in 'Moved Temporarily.' response body #21

Closed
mattaddy opened this issue Sep 5, 2012 · 6 comments
Closed

Comments

@mattaddy
Copy link

mattaddy commented Sep 5, 2012

First of all, excellent library. Thanks for your work.

Even the simplest route that redirects results in a Moved Temporarily. response body. For example,

app.get '/foobar', (req, res) ->
  res.redirect '/login'

app.get '/login', (req, res) ->
  res.render "#{__dirname}/views/login"
request = require('supertest')
app     = require('../../app')

describe '/foobar', ->
  it 'redirects to /login', (done) ->
    request(app)
      .get('/foobar')
      .expect('Login', done)
$ mocha test/my-test.coffee --compilers coffee:coffee-script

  Express server listening on port 3000
GET /foobar 302 3ms - 56
․

  ✖ 1 of 1 test failed:

  1) /foobar redirects to /login:
     Error: expected 'Login' response body, got 'Moved Temporarily. Redirecting to //127.0.0.1:3456/login'

One thing that looks odd is the different ports in the output Express server listening on port 3000, and the test Redirecting to //127.0.0.1:3456/login.

@tj
Copy link
Contributor

tj commented Sep 5, 2012

because you're redirecting :p redirects have been set to 0 because you should test what a route does (redirect and assert Location) not where it will end up

@tj tj closed this as completed Sep 5, 2012
@jensklose
Copy link

I understand the concept of that issue, but I don't want magic test results. ;)

My problem on api testing:
The http spec says:
"If a resource has been created on the origin server, the response SHOULD be 201 (Created) and contain an entity which describes the status of the request and refers to the new resource, and a Location header (see section 14.30)."

My test spec should:

Scenario: error message on duplicate item
    Given My request use http method POST to "/transactions"
    When I fill the field "author" with "test.once"
    When I fill the field "created" with "2015-08-24"
    When I call the api
    Then The response code equals 201
    When I call the api
    Then The response code equals 400
    Then The error message exists

with .redirects(0); it works, but only the 302 statusCode is in the response object.

Your suggestion to test what a route does is not the api and http-spec way:

" The response SHOULD include an entity containing a list of resource characteristics and location(s) from which the user or user agent can choose the one most appropriate. The entity format is specified by the media type given in the Content-Type header field."

@invernizzie
Copy link
Contributor

@jensklose, what @visionmedia suggested is actually asserting that the response was a redirect (status code 302) and also the Location header contents, which is the entity containing a list of resource characteristics and location(s) from which the user or user agent can choose the one most appropriate (in all implementations that I've seen, it was always just a URL and not always having a Content-Type header to match).

If you want a status code 201 to be sent, since a resource was created, just don't redirect!

@jensklose
Copy link

Sorry, but you should visit some RESTful APIs and not the quick and dirty CRUD earlies. ;)

see this discus to review your 302 decision

You could implement a "raw" mode request.rawRedirects()without statusCode morphing.

@invernizzie
Copy link
Contributor

What would you rather do in this case? Include 301 and 303? I think I misunderstood the point of your first comment here.

Oh and, please, don't go telling people what they don't know unless you want to turn a constructive discussion into a destructive argument. By doing that you're just being a dick. If you really know that much about REST and the interwebs, why is it that you didn't send a pull request to fix this yet?

@jensklose
Copy link

I must apologize to you on two counts. After investing some deeper look in my test case, your great library and the express api, I've found the fault on my site.

Express makes the response statusCode switch to 302 if I try a
accept('html') in combination with the field(key, value) method.
Switch to
accept('json') and send({key: "value"})
make it work and me dancing.

One way to test API calls with response code and redirect location:

        var test = superagent('post','http://localhost:3030/transactions');
        test.accept('json');
        test.redirects(0);
        test.send({author:"Test.Sampler"});
        test.end(function(err, res) {
            if (err) {
                throw err;
            }
//            console.log(res);
            res.status.should.eql(201);
            var matcher = new RegExp(uriPart);
            res.headers.location.should.match(matcher);
            callback();
        });

Thx for your work.

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

No branches or pull requests

4 participants