run: add hello-broken sample #1480
Conversation
googlebot
added
the
cla: yes
| RUN npm install --only=production | ||
|
|
||
| # Copy local code to the container image. | ||
| COPY . ./ |
There was a problem hiding this comment.
I don't think you need a forward slash here.
There was a problem hiding this comment.
While the previous practice was to use COPY . ., this is a newer practice I think we should use in COPY statements. The issue is COPY supports having multiple source arguments and one destination argument, but if the source arguments are:
- A directory
- More than one file
Then the destination needs to have the trailing slash otherwise it will error.
This is intended to future proof the use of the Dockerfile should a developer use it as a template.
|
Note the README here has a good amount of wording copied from other Run samples, I will revise all of them in follow-up to keep it in sync. |
run/hello-broken/package.json
Outdated
| "scripts": { | ||
| "start": "node index.js", | ||
| "test": "echo \"Error: no test specified\" && exit 0", | ||
| "e2e-test": "TARGET=Cloud test/runner.sh mocha test/system.test.js --timeout=20000" |
There was a problem hiding this comment.
It might be worth it to just put this script as test so you may npm test instead of npm run e2e-test.
There was a problem hiding this comment.
Out of scope of this PR.
There was a problem hiding this comment.
Actually, now that I see your point about npm test being a no-op, this bears some follow-up. I will file an issue.
There was a problem hiding this comment.
I've looked at the other samples that include script e2e-test in this repo as well as the cDPE test guide and squad notes.
If we consider this change out of scope, then I oppose adding this line then:
"test": "echo \"Error: no test specified\" && exit 0",We do specify a test in this repo. We don't specify a config for specifying a test as the error suggests.
If we're going to add this script:
"e2e-test": ...We should either consider using the Node way of npm test or fix the error message for npm test to be more accurate.
This adds a "broken" / difficult to troubleshoot sample for use in a troubleshooting tutorial.
This sample app is unusual in that it's is deliberately trying to show something not good as part of an exercise leading the developer through the full process of root cause analysis, fixing production, then improving the code.
Go variant: GoogleCloudPlatform/golang-samples#981