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

Testcase fix for epoll #3813

Merged
merged 1 commit into from
Aug 17, 2024
Merged

Testcase fix for epoll #3813

merged 1 commit into from
Aug 17, 2024

Conversation

tiif
Copy link
Contributor

@tiif tiif commented Aug 16, 2024 •
edited
Loading

Fixes #3811

This PR:

  • Fixed the error in epoll_ctl_del test
  • Simplified the logic in epoll_ctl_mod test
  • Added a new test to check if flag that we don't register won't trigger notification (double negation sounds a bit confusing here, feel free to suggest a better one ;) )
  • Use assert_eq(0) for epoll_ctl test

@tiif
Copy link
Contributor Author

tiif commented Aug 16, 2024 •
edited
Loading

7db8243 is a set of test that will pass real system. The only thing that we can do here is support EPOLLHUP flag. Another test is something that we discussed before.

EDIT: EPOLLHUP support is added in #3814

@tiif
Copy link
Contributor Author

tiif commented Aug 16, 2024

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Aug 16, 2024
@bors
Copy link
Contributor

bors commented Aug 17, 2024

☔ The latest upstream changes (presumably #3814) made this pull request unmergeable. Please resolve the merge conflicts.

@RalfJung
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Aug 17, 2024
@tiif
Apply test fix
de64092 
- Fix epoll_ctl_del test
- Simplified epoll_ctl_mod test and add test_no_notification_for_unregister_flag
- Use assert_eq(0) for epoll_ctl
@tiif
Copy link
Contributor Author

tiif commented Aug 17, 2024

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Aug 17, 2024
@tiif tiif mentioned this pull request Aug 17, 2024
Merged
@RalfJung
Copy link
Member

Looks good, thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Aug 17, 2024

📌 Commit de64092 has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 17, 2024

⌛ Testing commit de64092 with merge 85c5a8e...

@bors
Copy link
Contributor

bors commented Aug 17, 2024

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 85c5a8e to master...

@bors bors merged commit 85c5a8e into rust-lang:master Aug 17, 2024
8 checks passed
@bors bors mentioned this pull request Aug 17, 2024
Merged
@tiif tiif deleted the epollfix branch August 18, 2024 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees
No one assigned
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pass-dep/libc/libc-epoll.rs test does not pass when run against real system
4 participants
@tiif @bors @RalfJung @rustbot