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

sysfs: restore check for EINTR in poll test, fix Windows tests #1604

Merged
merged 1 commit into from
Jul 30, 2023

Conversation

evacchi
Copy link
Contributor

@evacchi evacchi commented Jul 30, 2023

EINTR used to be ignored when select was used, but the check was
deleted during the migration to poll: looks like this made some tests flaky.

Some time-based tests for Windows also seem flakier, so we try to make them slightly more robust.

Fixes #1601

Signed-off-by: Edoardo Vacchi evacchi@users.noreply.github.com

EINTR used to be ignored when `select` was used, but the check was
deleted during the migration to `poll`, but this might make
some tests flakier.

Some time-based tests also seem flakier, so we try to
make them slightly more robust.

Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
@evacchi evacchi changed the title sysfs: restore check for EINTR poll test, fix Window tests sysfs: restore check for EINTR in poll test, fix Window tests Jul 30, 2023
@evacchi evacchi changed the title sysfs: restore check for EINTR in poll test, fix Window tests sysfs: restore check for EINTR in poll test, fix Windows tests Jul 30, 2023
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

I recall once us looping on EINTR internally, such as Go does in functions that call os.ignoringEINTR, for reasons including avoiding excess host calls. I wonder if that's appropriate in osFile or not for File.Poll? Such an adjustment can be now or later or never

@codefromthecrypt codefromthecrypt merged commit d88286b into tetratelabs:main Jul 30, 2023
59 checks passed
@evacchi evacchi deleted the poll-test-eintr branch July 31, 2023 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Flakey test: Test_poll/should_wait_for_the_given_duration
2 participants