Clarity in Readme

[aaronik]

Hey RJ,

I'm updating express for my company and in the process updating supertest-session. There's a piece of the README I'm a bit confused on, maybe you could help me and then clarify the README if it's accurate and fix it if it isn't.

On L51 it looks like we can go through with a privileged request after a previous signin. But, our testSession from the example is repopulated in a beforeEach callback. If testSession is trampled each time, wouldn't the signed in session cookies get nuked for each one? If so, looks like the README should employ a before function instead of a beforeEach. If not, color me confused, and maybe the README could use a note on it, depending on from how deep a well my confusion comes.

Cheers,

-- Aaron

[rjz]

Good catch—thanks, @Aaronik! That spec definitely shouldn't pass as written.

I'd be very happy to merge a PR changing to a before block, or—if we want to keep state clean between assertions—nesting the 'logged-in' portion of the example inside a separate describe with appropriate setup.

[aaronik]

Cool :smile: Happy to do either! Any preference?

[rjz]

The separate block may be a little clearer for demonstrating session re-use? Something like:

describe('after authenticating session', function () {

  var authenticatedSession = null;

  beforeEach(function (done) {
    testSession.post('/signin')
      .send({ username: 'foo', password: 'password' })
      .expect(200)
      .end(function (err) {
        if (err) return done(err);
        authenticatedSession = testSession;
        return done();
      });
  });

  it('should get a restricted page', function (done) {
    authenticatedSession.get('/restricted')
      .expect(200)
      .end(done);
  });
});

[aaronik]

Hmmm.. have a branch going, but it's looking like I don't have access rights enough to create a new PR - I sure hope I'm not doing something wrong :blush:

$ git push origin readme/signin/22 

ERROR: Permission to rjz/supertest-session.git denied to Aaronik.
fatal: Could not read from remote repository.

And of course

$ git remote -v

origin  git@github.com:rjz/supertest-session.git (fetch)
origin  git@github.com:rjz/supertest-session.git (push)

[rjz]

@Aaronik looks like it's trying to push to this repo. If you fork to aaronik/supertest-session and open the pull request from the fork (using the web UI) we should be in business!

[aaronik]

Aight check this: https://github.com/rjz/supertest-session/pull/23 Never done this type of PR before so if I'm doing it wrong, very happy to continue massaging the process if you don't mind holding my hand through it :)