Skip to content
This repository

Fixed redirect handling. Includes tests #42

Open
wants to merge 2 commits into from

5 participants

Reuven V. Gonzales Jonathan Ong Andrea Rossi Garrett Johnson TJ Holowaychuk
Reuven V. Gonzales

I was using supertest when developing the other day and realized that redirects could not be examined. This fixes that issue.

TJ Holowaychuk visionmedia commented on the diff
lib/test.js
@@ -120,7 +120,7 @@ Test.prototype.expect = function(a, b, c){
120 120 Test.prototype.end = function(fn){
121 121 var self = this;
122 122 var end = Request.prototype.end;
123   - end.call(this, function(res){
  123 + end.call(this, function(err, res){
1
TJ Holowaychuk Owner

we should check this error, ignoring it wont be good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reuven V. Gonzales ravenac95 Handle the error on the `end` callback
Added an additional test to ensure it works.
a341959
Reuven V. Gonzales

Sorry about that! I just fixed it up. Oddly enough the error is still handled somehow without the logic that's now in test.js.

Jonathan Ong

@visionmedia any plans for redirect support?

Andrea Rossi

+1 for jonathanong request

Garrett Johnson
Collaborator

By redirect support, are you just wanting a way to assert against each redirect?

Jonathan Ong

i don't exactly remember as i went around it. i'm pretty sure resolving redirects.

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

Showing 2 unique commits by 1 author.

Dec 05, 2012
Reuven V. Gonzales ravenac95 Fixed redirect handling. Includes tests c009623
Dec 06, 2012
Reuven V. Gonzales ravenac95 Handle the error on the `end` callback
Added an additional test to ensure it works.
a341959
This page is out of date. Refresh to see the latest.

Showing 2 changed files with 68 additions and 2 deletions. Show diff stats Hide diff stats

  1. +4 1 lib/test.js
  2. +64 1 test/supertest.js
5 lib/test.js
@@ -120,7 +120,10 @@ Test.prototype.expect = function(a, b, c){
120 120 Test.prototype.end = function(fn){
121 121 var self = this;
122 122 var end = Request.prototype.end;
123   - end.call(this, function(res){
  123 + end.call(this, function(err, res){
  124 + if(err) {
  125 + return self.emit('error', err);
  126 + }
124 127 self.assert(res, fn);
125 128 });
126 129 return this;
65 test/supertest.js
@@ -3,7 +3,8 @@ var request = require('..')
3 3 , https = require('https')
4 4 , fs = require('fs')
5 5 , path = require('path')
6   - , express = require('express');
  6 + , express = require('express')
  7 + , should = require('should');
7 8
8 9 describe('request(app)', function(){
9 10 it('should fire up the app on an ephemeral port', function(done){
@@ -119,6 +120,68 @@ describe('request(app)', function(){
119 120 .expect(302, done);
120 121 })
121 122
  123 + it('should redirect many times', function(done){
  124 + var app = express();
  125 +
  126 + app.get('/', function(req, res){
  127 + res.redirect('/redirect1');
  128 + });
  129 +
  130 + app.get('/redirect1', function(req, res){
  131 + res.redirect('/redirect2');
  132 + });
  133 +
  134 + app.get('/redirect2', function(req, res){
  135 + res.redirect('/redirect3');
  136 + });
  137 +
  138 + app.get('/redirect3', function(req, res){
  139 + res.send(200, 'Redirected 3 times');
  140 + });
  141 +
  142 + request(app)
  143 + .get('/')
  144 + .redirects(5)
  145 + .end(function(err, res) {
  146 + res.should.have.status(200);
  147 + res.redirects.should.have.length(3);
  148 + res.text.should.equal('Redirected 3 times');
  149 + done()
  150 + });
  151 + })
  152 +
  153 + it('should throw an error during redirects', function(done){
  154 + var app = express();
  155 +
  156 + app.get('/', function(req, res){
  157 + res.redirect('/redirect1');
  158 + });
  159 +
  160 + app.get('/redirect1', function(req, res){
  161 + res.redirect('/redirect2');
  162 + });
  163 +
  164 + app.get('/redirect2', function(req, res){
  165 + setTimeout(function() {
  166 + res.send(200, 'Should not see me!');
  167 + }, 200);
  168 + });
  169 +
  170 + var timeout = 10;
  171 +
  172 + request(app)
  173 + .get('/')
  174 + .timeout(timeout)
  175 + .redirects(5)
  176 + .on('error', function(err) {
  177 + err.timeout.should.equal(timeout);
  178 + done();
  179 + })
  180 + .end(function(err, res) {
  181 + should.fail('There was supposed to be an error');
  182 + });
  183 + })
  184 +
122 185 describe('.expect(status[, fn])', function(){
123 186 it('should assert the response status', function(done){
124 187 var app = express();

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.