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.

Sign up for GitHub

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

Jump to bottom

os: TestKillFindProcess failures with "Test process succeeded, but expected to fail" #44131

Closed
bcmills opened this issue Feb 5, 2021 · 5 comments
Closed

os: TestKillFindProcess failures with "Test process succeeded, but expected to fail" #44131

bcmills opened this issue Feb 5, 2021 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.18

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 5, 2021 

--- FAIL: TestKillFindProcess (1.78s)
    os_test.go:2199: Test process succeeded, but expected to fail
FAIL
FAIL	os	5.004s

2020-08-17T20:06:49-e6d0bd2/darwin-amd64-nocgo
2019-11-02T21:51:23-7955ece/aix-ppc64
2019-11-01T16:05:22-8d45e61/aix-ppc64
2019-09-01T10:08:53-99df76f/solaris-amd64-oraclerel

See previously #43722.
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 5, 2021
@bcmills bcmills added this to the Backlog milestone Feb 5, 2021
@bcmills
Copy link
Contributor Author

bcmills commented Feb 5, 2021

https://storage.googleapis.com/go-build-log/baa2ea49/android-amd64-emu_2a586092.log (a TryBot)

@bcmills
Copy link
Contributor Author

bcmills commented Feb 9, 2022

greplogs --dashboard -md -l -e 'FAIL: TestKillFindProcess .*(?:\n .*)* Test process succeeded, but expected to fail' --since=2021-02-06

2022-02-08T19:03:52-9e0de1f/dragonfly-amd64
2021-09-24T04:32:28-9ce601d/dragonfly-amd64
2021-09-09T21:28:56-c981874/dragonfly-amd64

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/386174 mentions this issue: os: eliminate arbitrary sleep in Kill tests

@bcmills
Copy link
Contributor Author

bcmills commented Feb 16, 2022

greplogs --dashboard -md -l -e 'FAIL: TestKillFindProcess .*(?:\n .*)* Test process succeeded, but expected to fail' --since=2022-02-09

2022-02-15T21:18:59-08ed488/dragonfly-amd64

Looks like an easy fix, too.

@bcmills bcmills self-assigned this Feb 16, 2022
@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure. labels Feb 16, 2022
@bcmills bcmills modified the milestones: Backlog, Go1.18 Feb 16, 2022
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 16, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/386196 mentions this issue: [release-branch.go1.18] os: eliminate arbitrary sleep in Kill tests

gopherbot pushed a commit that referenced this issue Feb 16, 2022
@dmitshur
[release-branch.go1.18] os: eliminate arbitrary sleep in Kill tests
e70ee95 
The test spawned a subprocess that arbitrarily slept for one second.
However, on some platforms, longer than one second may elapse between
starting the subprocess and sending the termination signal.

Instead, the subprocess now closes stdout and reads stdin until EOF,
eliminating the need for an arbitrary duration. (If the parent test
times out, the stdin pipe will break, so the subprocess still won't
leak forever.)

This also makes the test much faster in the typical case: since it
uses synchronization instead of sleeping, it can run as quickly as the
host OS can start and kill the process.

Updates #44131.

Change-Id: I9753571438380dc14fc3531efdaea84578a47fae
Reviewed-on: https://go-review.googlesource.com/c/go/+/386174
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit eaf0405)
Reviewed-on: https://go-review.googlesource.com/c/go/+/386196
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@rsc rsc unassigned bcmills Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
2 participants
@bcmills @gopherbot