Skip to content
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

runtests: add option -u to error on server unexpectedly alive #7180

Closed

Conversation

mback2k
Copy link
Member

@mback2k mback2k commented Jun 2, 2021

Let's try to actually handle the server unexpectedly alive
case by first making them visible on CI builds as failures.

@mback2k mback2k self-assigned this Jun 2, 2021
@bagder bagder added CI Continuous Integration tests labels Jun 2, 2021
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch 3 times, most recently from a6930d0 to f2e60fd Compare July 19, 2021 10:06
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from f2e60fd to e37e3ad Compare July 22, 2021 10:45
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from e37e3ad to a854a9a Compare July 23, 2021 18:54
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from a854a9a to cd993f6 Compare July 25, 2021 18:04
mback2k added a commit to mback2k/curl that referenced this pull request Jul 29, 2021
Let's try to actually handle the server unexpectedly alive
case by first making them visible on CI builds as failures.

Closes curl#7180
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from cd993f6 to 85daa74 Compare July 29, 2021 06:13
@mback2k
Copy link
Member Author

mback2k commented Aug 7, 2021

This helped discover the issues fixed with #7481, #7482 and #7506 and will probably be merged after #7530.

mback2k added a commit to mback2k/curl that referenced this pull request Aug 15, 2021
Let's try to actually handle the server unexpectedly alive
case by first making them visible on CI builds as failures.

Closes curl#7180
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from 85daa74 to a3ff01c Compare August 15, 2021 10:12
mback2k added a commit to mback2k/curl that referenced this pull request Aug 17, 2021
Let's try to actually handle the server unexpectedly alive
case by first making them visible on CI builds as failures.

Closes curl#7180
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from a3ff01c to 389edbb Compare August 17, 2021 19:46
mback2k added a commit to mback2k/curl that referenced this pull request Aug 19, 2021
Let's try to actually handle the server unexpectedly alive
case by first making them visible on CI builds as failures.

Closes curl#7180
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from 389edbb to 3b3799a Compare August 19, 2021 10:28
@mback2k mback2k requested a review from bagder August 19, 2021 10:29
@mback2k mback2k marked this pull request as ready for review August 19, 2021 10:29
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message needs an update. I suppose the reason for this option is to detect and provide a way to debug problems with killing off test servers?

Let's try to actually handle the server unexpectedly alive
case by first making them visible on CI builds as failures.

This is needed to detect issues with killing of the test
servers completely including nested process chains with
multiple PIDs per test server (including bash and perl).

On Windows/cygwin platforms this is especially helpful with
debugging PID mixups due to cygwin using its own PID space.

Reviewed-by: Daniel Stenberg
Closes curl#7180
@mback2k mback2k force-pushed the fail-unexpected-testserver-alive branch from 3b3799a to 60a6d1d Compare August 20, 2021 10:58
@mback2k mback2k requested a review from bagder August 20, 2021 10:59
@mback2k
Copy link
Member Author

mback2k commented Aug 20, 2021

Thanks for the feedback @bagder. I just rebased and pushed an extended commit message.

@mback2k mback2k closed this in 60efeb1 Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration tests
Development

Successfully merging this pull request may close these issues.

None yet

2 participants