SLM-85: Added tests using supertest that spin up and test the express application #222
Conversation
| @@ -59,7 +59,7 @@ const redirect = () => | |||
| 'Content-Type': 'text/html', | |||
| Location: 'http://localhost:3007/sign-in/callback?code=codexxxx&state=stateyyyy', | |||
| }, | |||
| body: '<html><head><title>SignIn page</title></head><body><h1>Sign in</h1><span class="govuk-visually-hidden" id="sign-in"></span></body></html>', | |||
There was a problem hiding this comment.
The span in the markup with the pageId has been changed, so these mocks (for pages that are not in our codebase) need to change
| @@ -23,7 +23,7 @@ export default abstract class Page { | |||
| } | |||
|
|
|||
| checkOnPage = (): void => { | |||
| cy.get(`#${this.pageId}`).should('exist') | |||
| cy.get('#pageId').should('have.attr', 'data-qa').should('equal', this.pageId) | |||
| } | |||
There was a problem hiding this comment.
The span with the pageId has changed in the template - see below 👇
| @@ -63,7 +63,8 @@ | |||
| "json", | |||
| "node", | |||
| "ts" | |||
| ] | |||
| ], | |||
| "testRunner": "jest-jasmine2" | |||
There was a problem hiding this comment.
We need to include the jasmine runner to be able to use fail in tests - its basically a bug in jest-circus - see github issue and the line in the description:
Can circumvent in 27.x with testRunner: "jest-jasmine2" in jest.config.js
| describe('Select envelope size', () => { | ||
| const superTest = new SuperTestWrapper() | ||
|
|
||
| beforeEach(async () => { |
There was a problem hiding this comment.
I've added a test that requires the user to be authenticated. The test spec itself could do with some more functional tests adding (ie. submitting the envelope size etc), but I wanted to make sure we could send requests as an authenticated user
|
|
||
| jest.mock('redis', () => jest.requireActual('redis-mock')) | ||
|
|
||
| export default class SuperTestWrapper { |
There was a problem hiding this comment.
This is the main class you are going to use when writing supertest tests - basically new up a SuperTestWrapper and use it's request property to issue requests.
It also has helper methods to make the test context authenticated or unauthenticated (need to add a method to authenticate as a mail room user, but that will come ....)
| } | ||
|
|
||
| unauthenticated = async () => { | ||
| await this.request.get('/link/request-link?force=true') // log out legal sender user |
There was a problem hiding this comment.
The easiest way to be unauthenticated is to hit the Legal Sender sign out link - it doesnt matter whether you were previously logged in as a LS or not; it will log you out 👍
As and when we have tests that authenticate as a mail room user we'll need to change this to log them out as well
| return new SupertestAssertions(response) | ||
| } | ||
|
|
||
| class SupertestAssertions { |
There was a problem hiding this comment.
This is effectively our DSL for making assertions. All methods are chainable and is modelled heavily on assertJ (mainly because I couldnt work out how to write my own expect method that didnt conflict with jest's expect)
Also, in most assertion methods I have a try/catch with a fail (re: the change to package.json to bring in jasmine)
Its a bit long winded but its because jest's expect method doesnt support failure messages 😢
Long winded? yes; but its nicely abstracted away into this class, so test methods dont need to worry about it
| <span class="govuk-visually-hidden" id="{{ pageId }}"></span> | ||
| <span class="govuk-visually-hidden" id="pageId" data-qa="{{ pageId }}"></span> |
There was a problem hiding this comment.
I've left the original span there as well for the time being because there are nunjucks tests that use the old element , and this PR is big enough already!
| } | ||
|
|
||
| hasAllErrors = (...expected: string[]): SupertestAssertions => { | ||
| const actual = this.$('.govuk-error-summary__list').text() |
There was a problem hiding this comment.
Minor point - might be better to split this by newlines before testing? Meh.
| @@ -35,8 +35,9 @@ export default class VerifyLinkController { | |||
| req.flash('errors', [ | |||
| { href: '#email', text: 'The link you used is no longer valid. Request a new one to sign in.' }, | |||
| ]) | |||
| res.redirect('/link/request-link') | |||
| return res.redirect('/link/request-link') | |||
There was a problem hiding this comment.
do we also need this on line 28? bit confused as to how this ever worked!
There was a problem hiding this comment.
I'll explain it tomorrow, but basically it worked more by luck than judgement, by virtue of the fact that the error message rendered in the flash object for both use cases is the same (lines 26 and 36).
If they had different error messages we would have noticed this bug sooner.
The only reason I noticed it here is that res.render was being called twice (I'll explain why tomorrow) and the supertest http client (superagent) complained bitterly that the response had already been written to and that it couldnt do it again!
| }) | ||
|
|
||
| it('should render page to request a link', async () => { | ||
| const response = await superTest.request // |
There was a problem hiding this comment.
Does the // at the end mean anything?
There was a problem hiding this comment.
No, it's just a way I can shoehorn my desire to lay out the code into a more readable manner (IMHO) rather than being at the whim of the draconian prettier rules 😁
To be fair this particular instance is not that offensive because there is only one method call being chained:
const response = await superTest.request //
.get('/link/request-link')
Without the comment markers prettier would force me to do:
const response = await superTest.request.get('/link/request-link')
That's no so bad, but there are other cases below where I chain more method calls:
const response = await superTest.request //
.post('/link/request-link')
.send({ email: '[email protected]' })
If prettier had it's way it would force me to do:
const response = await superTest.request.post('/link/request-link').send({ email: '[email protected]' })
Horrible 🤮
No description provided.