-
Notifications
You must be signed in to change notification settings - Fork 1
Feature/12938 investigate how to write integration tests #358
Feature/12938 investigate how to write integration tests #358
Conversation
@KeithLyonMastek, looks like there are some conflicts that need resolving. |
Story: AB#12937 Task: AB#12938
122be8b
to
5831a25
Compare
…/public-browse into feature/12938-investigate-how-to-write-integration-tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems good to me.
91f76ac
to
e9b2157
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of small things.
app/includes/manifest.json
Outdated
"showLegalPanel": true | ||
"showLegalPanel": true, | ||
"solutionsVacination": "/solutions/vaccinations", | ||
"solutionsCovid": "/solutions/covid19" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just needs a trailing newline.
package.json
Outdated
"nock": "^13.0.11", | ||
"nunjucks": "^3.2.3", | ||
"nunjucks-date-filter": "^0.1.1", | ||
"path": "^0.12.7", | ||
"serve-favicon": "^2.5.0", | ||
"testcafe": "^1.11.0", | ||
"testcafe-reporter-nunit": "^0.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't nock
and the TestCafe libraries be dev dependencies?
Can you also double-check that the dependencies are up-to-date? (Just thinking of minor updates – might be best to do updates like the component library update in a separate PR.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have moved them into devDependencies. I ran 'npm update' but does not seem to have done much so perhaps we are up to date. Is there a flag you typically run it with that I am missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's a bug in the latest version of npm, which means that it doesn't update package.json
, only package-lock.json
. There's an open issue (708) for it in the npm repo.
When there's only a few I tend to just do npm install x@latest
for the relevant dependencies – obviously a pain if there are a lot though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmm, that is a bad bug, running 'npm outdated' does not seem to return a list either.
…/public-browse into feature/12938-investigate-how-to-write-integration-tests
e9b2157
to
33b39a5
Compare
Remove duplicate tests for header & footer in PB, replace with cheerio snapshot