-
-
Notifications
You must be signed in to change notification settings - Fork 583
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
fix: clean up test project on signal, fixes #6111 #6119
Conversation
Download the artifacts for this pull request:
See Testing a PR. |
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.
Tested
- Ctrl-C during run, it should clean up or give a good try
- It should completely clean up after a normal run
- If not internet is available, or google.com not accessible, the failure should show
Thanks!
Minor notes:
-
There is one more hard-coded port
60004
. -
$router_http_port
and$router_https_port
are not used in the script:
ddev/cmd/ddev/cmd/scripts/test_ddev.sh
Lines 83 to 84 in d358586
router_http_port=$(ddev config global -j | docker run -i --rm ddev/ddev-utilities jq -r '.raw."router-http-port"' 2>/dev/null) | |
router_https_port=$(ddev config global -j | docker run -i --rm ddev/ddev-utilities jq -r '.raw."router-https-port"' 2>/dev/null) |
- A typo in
healtcheck
>healthcheck
ddev/cmd/ddev/cmd/scripts/test_ddev.sh
Line 164 in d358586
printf "============= ddev-${PROJECT_NAME}-web healtcheck run =========\n" && \
Thanks for the careful review!
|
Thanks for your eagle eye! |
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.
Tested again, all good 👍
The Issue
ddev debug test
may have conflicts when running sequentially (andddev debug testcleanup
have no effect) #6111How This PR Solves The Issue
Manual Testing Instructions