Added argument passing to invalidate method #1093
Conversation
|
What's your use case for this? This would also need a test ;) |
|
I have certain cases in which I know the backend will revoke the session (Platform-Update, etc..) and need to invalidate it on client side without doing a server call. Also you can send additional data on the logout (leaving message) or build advanced logic into the invalidate call. I'll write tests for this in the next days 👍 |
|
@leuprechtroman i just did a pull request to your fork with a test case for this feature. |
mainmatter#1093 test related to your update in the invalidate with params
leuprechtroman
force-pushed
the
master
branch
from
6299075 to
43f6293
Compare
|
@hajjem-ayoub Thank you so much! I updated my repo, included the tests and made everything pass the tests. |
tests/unit/internal-session-test.js
Outdated
| @@ -385,7 +385,17 @@ describe('InternalSession', () => { | |||
| return session.authenticate('authenticator'); | |||
| }); | |||
|
|
|||
| describe('when the authenticator resolves invaldiation', () => { | |||
| describe('when the authenticator resolves invalidation with params', () => { | |||
| it('when some data is sent with invalidate', () => { | |||
There was a problem hiding this comment.
Test descriptions should always start with a verb actually, e.g. it("passes the params on to the authenticator's invalidate method".
tests/unit/internal-session-test.js
Outdated
| let invalidateSession = sinon.spy(authenticator, 'invalidate'); | ||
| let param = { some: 'random data' }; | ||
| session.invalidate(param); | ||
| invalidateSession.restore(); |
There was a problem hiding this comment.
You'll always want to set up spies (and especially restore the original methods) in beforeEach/afterEach hooks so you can be sure spies will always be cleaned up reliably, even if an exception is thrown in the test case.
tests/unit/internal-session-test.js
Outdated
| let param = { some: 'random data' }; | ||
| session.invalidate(param); | ||
| invalidateSession.restore(); | ||
| sinon.assert.calledWith(invalidateSession, session.get('authenticated'), param); |
There was a problem hiding this comment.
ESA generally uses chai (and sinon-chai) for assertions, e.g. in this case:
expect(authenticator.invalidate).to.have.been.calledWith(session.get('authenticated'), param);
|
Thanks for your feedback, I've adapted everything according to it! |
tests/unit/internal-session-test.js
Outdated
| @@ -385,7 +385,25 @@ describe('InternalSession', () => { | |||
| return session.authenticate('authenticator'); | |||
| }); | |||
|
|
|||
| describe('when the authenticator resolves invaldiation', () => { | |||
| describe('when the authenticator resolves invalidation with params', () => { | |||
There was a problem hiding this comment.
Sorry, oversaw that: this is the wrong text actually. This block is not actually about the authenticator resolving or rejecting, instead you'd want something like describe('when arguments are passed', () => {.
I saw, that the authenticate method passes the arguments into the authenticator. But the invalidate method does not do that. So I simply added the arguments into the method call to enable the same behaviour as seen in the authenticate method.