[terra-toolkit-docs] Update terra-functional-testing upgrade guide #648
Conversation
benbcai
requested review from
dkasper-was-taken,
mjhenkes,
ryanthemanuel and
tbiethman
as code owners
| @@ -317,7 +325,13 @@ npm run start-prod | |||
| npm run start-static | |||
| ``` | |||
|
|
|||
| Note: If you don't have some of these commands it is not necessary to add them. Just ensure the commands you do have are working as expected. All of the `tt-serve` commands should be updated to use webpack-dev-server or the terra express server | |||
| Note: If you don't have some of these commands it is not necessary to add them. If you changed from `tt-serve-static` to use the `express-server` command, be sure to also install the `@cerner/terra-open-source-scripts` package that provides this command. Just ensure the commands you do have are working as expected. All of the `tt-serve` commands should be updated to use webpack-dev-server or the terra express server. | |||
There was a problem hiding this comment.
I wonder if we should split the express server out of open source scripts.. We didn't originally intend for internal teams to use that package.
There was a problem hiding this comment.
Move it to terra-functional-testing?
There was a problem hiding this comment.
I think we should move it to terra-functional-testing. We also forgot to add the @cerner/terra-open-source-scripts dependency when we did the upgrade in our other projects so we would need to go back to add the dependency if we don't split it.
There was a problem hiding this comment.
I'm onboard with moving it to terra-functional testing
There was a problem hiding this comment.
Ok. I'll move it as part of this PR and remove this from the doc.
…ra-functional-testing to eliminate the need to depend on terra-open-source-scripts to use express-server.
| @@ -2,6 +2,9 @@ | |||
|
|
|||
| ## Unreleased | |||
|
|
|||
| * Removed | |||
There was a problem hiding this comment.
Technically non passive. Though for our uses, I'm not sure if it will affect us since for us most, if not all, packages that use the open source scripts, also use terra-functional testing.
There was a problem hiding this comment.
I searched in github for all the places that depend on @cerner/terra-open-source-scripts. There are a few but they all already also depend on @cerner/terra-functional-testing so this should continue to work for them.
| @@ -7,6 +7,5 @@ module.exports = async (options) => { | |||
| site: options.site, | |||
| }); | |||
|
|
|||
| await server.createApp(); | |||
There was a problem hiding this comment.
I removed the call to createApp() because it is already being called inside start(). This is also to match AssetServerService where calling createApp() is not needed.
There was a problem hiding this comment.
I was gonna ask about that :)
| @@ -1,4 +1,4 @@ | |||
| const ExpressServer = require('./ExpressServer'); | |||
| const ExpressServer = require('../../express-server'); | |||
There was a problem hiding this comment.
This will now use the express-server that is currently in terra-functional-testing.
Summary
Closes #638
@cerner/terra