-
Notifications
You must be signed in to change notification settings - Fork 731
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: liveness tests #1456
refactor: liveness tests #1456
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1456 +/- ##
==========================================
+ Coverage 10.44% 15.68% +5.23%
==========================================
Files 11 10 -1
Lines 1082 1371 +289
==========================================
+ Hits 113 215 +102
- Misses 964 1145 +181
- Partials 5 11 +6 |
ah just realized this is failing lint chcker cause you removed all the |
if: env.GIT_DIFF | ||
- name: Test Local Network Liveness | ||
run: | | ||
sleep 3m |
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.
why sleep ? and sleep so long ?
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.
ignite
start a blocking process where it runs a chain, so we run the process in the background and pipe both stdout and stderr to a file. ignite
takes a bit to startup (builds app, builds proto, etc...), so the 3m is just a safe "buffer" to wait for ignite
to actually start the chain.
|
||
docker_containers=($(docker ps -q -f name=umeed --format='{{.Names}}')) | ||
|
||
while [ ${CNT} -lt $ITER ]; do |
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.
is $ITER
used for only counting a max looping ? if this way, can we just write while [ ${CNT} -lt $(expr ${NUMBLOCKS} + 50) ]; do
then we do not need to pass the arg ITER=$1
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.
We've had this script in the SDK for years at this point. We could probably clean it up. I'm indifferent to it as it's pretty simple.
I thought it was accidentally removed, so i add the |
* updates * updates * updates * updates * updates * fix broken link * add missing file Co-authored-by: billy rennekamp <[email protected]> Co-authored-by: Yaru Wang <[email protected]>
* updates * updates * updates * updates * updates * fix broken link * add missing file Co-authored-by: billy rennekamp <[email protected]> Co-authored-by: Yaru Wang <[email protected]>
Build & Test