CI: add a test case for create account #44
Conversation
test/new_project.sh
Outdated
| @@ -8,3 +8,8 @@ npm install | |||
| npm uninstall near-shell | |||
| npm install ../ | |||
| NODE_ENV=development npm run test | |||
| timestamp=$(date +%s) | |||
There was a problem hiding this comment.
let's put this into separate script, e.g. test/create_account.sh?
There was a problem hiding this comment.
or maybe have a setup script which sets up new project and then run few different scenarios in separate scripts after that:
- run tests
- create account
- deploy
package.json
Outdated
| @@ -6,7 +6,7 @@ | |||
| "scripts": { | |||
| "build": "echo \"Error: not implemented\" && exit 1", | |||
| "deploy": "echo \"Error: not implemented\" && exit 1", | |||
| "test": "./test/new_project.sh" | |||
| "test": "./test/new_project.sh; ./test/create_account.sh" | |||
There was a problem hiding this comment.
I think you need to use && instead of ; here, otherwise npm might not return error code on ./test/new_project.sh failure.
There was a problem hiding this comment.
I just tested it with a purposefully invalid account id, and it does in fact fail the test with ;
&& does not work at all however.
There was a problem hiding this comment.
bash-3.2$ false ; true
bash-3.2$ echo $?
0
bash-3.2$ false && true
bash-3.2$ echo $?
1
bash-3.2$ true ; false
bash-3.2$ echo $?
1
I just tested it with a purposefully invalid account id, and it does in fact fail the test with ;
it fails because last step failed. It won't fail if ./test/new_project.sh fails
&& does not work at all however.
may you elaborate? It does work as expected in bash (see above). Maybe ./test/new_project.sh returns error somehow?
There was a problem hiding this comment.
FAIL src/test.js
Test suite failed to run
InternalServerError: Internal Server Error
at sendJson (../node_modules/nearlib/internal/send-json.js:12:15)
I'd prefer to not spend cycles to investigate
There was a problem hiding this comment.
I'd prefer to not spend cycles to investigate
so our goal is to have npm run test exit with error code when either of test scripts fails.
Using ; doesn't achieve this (see bash transcript above), npm run test would be successful if ./test/new_project.sh fails.
I think it's reasonable requirement that npm run test should fail when ./test/new_project.sh fails.
Just to be sure I checked that npm behavior is the same by modifying test script to be false; true:
➜ nearlib git:(master) ✗ npm run test && echo SUCCESS <<<
> nearlib@0.7.1 test /Users/vg/Documents/nearlib
> false; true
SUCCESS
I'd prefer to not spend more cycles explaining why this is a bug. I can of course also fix it myself, similar like I had to do to near-shell (e9ce691) cause CI would be completely happy with failed deploy, etc otherwise.
There was a problem hiding this comment.
Every time we get "Internal Server Error" without any details - that's a good indicator to actually investigate. Because that means that developer using this will be totally lost with this error.
There was a problem hiding this comment.
And for sure "&&" should be used in any bash environment like testing configs (note, in fish && doesn't work)
| @@ -0,0 +1,6 @@ | |||
| #!/bin/sh | |||
There was a problem hiding this comment.
It's best to do set -ex first (see more here https://explainshell.com/explain?cmd=set+-ex). -x is kinda matter of situation, but without -e script won't fail CI properly.
Removed colors cause they don't play well with set -x
No description provided.