WS-1347: Add local command for int test - Next.js#13875
WS-1347: Add local command for int test - Next.js#13875shayneahchoon merged 9 commits intolatestfrom
Conversation
| ); | ||
|
|
||
| child.on('exit', resolve); | ||
| const statusCommand = `sleep 20 && curl http://localhost:${portNumber}${pathname}`; |
There was a problem hiding this comment.
I've changed wait-on to curl since I couldn't get it to work properly on local.
package.json
Outdated
| "prepare": "husky install", | ||
| "preinstall": "node scripts/check-package-manager.js", | ||
| "start": "NODE_ENV=production node --max-old-space-size=3500 build/server.js", | ||
| "fallbackStart": "yarn dev", |
There was a problem hiding this comment.
The commands between the 2 apps have different behaviours. The Express one will start the app in development mode, wheras the Next one will attempt to start it in a production build. Is that intentional?
There was a problem hiding this comment.
the fallbackStart command on the express app isn't needed and is pretty irrelevant. I just put it there for consistency. The main issue is to do with the NextJS app which does benefit from the added fallbackStart command as some machines don't include the standalone server bundle.
There was a problem hiding this comment.
Happy to change the fallbackStart on express to something else however.
There was a problem hiding this comment.
I guess as well if isDev is true and yarn dev is used and fails, then the fallback will also fail because there is no production build for it to start?
There was a problem hiding this comment.
I've updated it at the script level to re-run dev if the flag is selected.
There was a problem hiding this comment.
Sorry not really sure I follow. Where is that re-run performed?
There was a problem hiding this comment.
Soz, I changed it here: https://github.com/bbc/simorgh/pull/13875/changes#diff-fb36cbd39b3e10f55d949c34da923ca9621e8cf10d44c67861b623cec8295043R52
So the new flow is as follows:
Next.js normal: yarn start -> fallback to yarn next start -p 7081
Express normal: yarn start -> fallback to yarn start
dev mode for both: yarn dev -> fallback to yarn dev
There was a problem hiding this comment.
Is there a danger in using dev mode for integration tests? We have snapshots that could potentially change as the build is bundled differently between dev and production, so I'd imagine Emotion class names would be different.
There was a problem hiding this comment.
I personally don't know anyone that uses dev mode at the moment, but there is a flag for it in case anyone wants to. From the way I've used it the normal commands are most reliable and consistent across on local and the github actions.
There was a problem hiding this comment.
Fair enough. I'm still not entirely sure what these changes help with in regards to failed test starts.
next start doesn't work with output: standalone, so I'd be afraid we'd be doing something that isn't quite compatible with our setup:
"next start" does not work with "output: standalone" configuration. Use "node .next/standalone/server.js" instead.
4 participants
Resolves JIRA: https://bbc.atlassian.net/browse/WS-1347
Summary
Adds a fallback command for integration tests and restructures promises to include an appropriate reject statement should start ups fail.
Code changes
yarn starton the Next.js app falls back toyarn next start -p 7081.Testing
Useful Links