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

Fix mq tests on NetBSD and DragonFly #1624

Merged
merged 1 commit into from
Jan 2, 2022
Merged

Conversation

rtzoeller
Copy link
Collaborator

NetBSD (and DragonFly, which borrows its implementation) include additional flags beyond O_NONBLOCK in MqAttr, such as the flags passed to mq_open(). Modify the mq tests to validate at least the expected flags are set, but don't require strict equality on these platforms.

Verified these tests pass on DragonFly and NetBSD locally.

#[cfg(not(any(target_os = "dragonfly", target_os = "netbsd")))]
{
let new_attr_get = mq_getattr(mqd).unwrap();
assert_ne!(new_attr_get, new_attr);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Presumably we could enable this assert for NetBSD and DragonFly as well, but writing the negation of assert_attr_eq is just painful enough (and IMO low-value enough) that I'd prefer not to do it. :)

NetBSD (and DragonFly, which borrows its implementation) include
additional flags beyond O_NONBLOCK in MqAttr, such as the flags
passed to mq_open().
@rtzoeller
Copy link
Collaborator Author

rtzoeller commented Jan 2, 2022

Once this and rust-lang/libc#2610 are submitted (and it makes it into a libc release used by nix), the enabled tests should be 100% passing on DragonFly. 🎉

@asomers
Copy link
Member

asomers commented Jan 2, 2022

bors r+

bors bot added a commit that referenced this pull request Jan 2, 2022
1624: Fix mq tests on NetBSD and DragonFly r=asomers a=rtzoeller

NetBSD (and DragonFly, which borrows its implementation) include additional flags beyond O_NONBLOCK in MqAttr, such as the flags passed to mq_open(). Modify the mq tests to validate _at least_ the expected flags are set, but don't require strict equality on these platforms.

Verified these tests pass on DragonFly and NetBSD locally.

Co-authored-by: Ryan Zoeller <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 2, 2022

Build failed:

@rtzoeller
Copy link
Collaborator Author

bors retry

bors bot added a commit that referenced this pull request Jan 2, 2022
1624: Fix mq tests on NetBSD and DragonFly r=asomers a=rtzoeller

NetBSD (and DragonFly, which borrows its implementation) include additional flags beyond O_NONBLOCK in MqAttr, such as the flags passed to mq_open(). Modify the mq tests to validate _at least_ the expected flags are set, but don't require strict equality on these platforms.

Verified these tests pass on DragonFly and NetBSD locally.

Co-authored-by: Ryan Zoeller <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 2, 2022

Build failed:

@rtzoeller
Copy link
Collaborator Author

Caused by:
  process didn't exit successfully: `/tmp/cirrus-ci-build/target/x86_64-unknown-freebsd/debug/deps/test-9b88884763bbe54f` (signal: 14, SIGALRM: alarm clock)

@asomers any ideas on this FreeBSD failure?

@rtzoeller
Copy link
Collaborator Author

AFAICT the failure is due to some sort of race condition between the newly minted test_timer::alarm_fires and the existing alarm tests. It also reproduces intermittently on NetBSD locally when running the tests in the same test run.

@asomers
Copy link
Member

asomers commented Jan 2, 2022

I can't reproduce the failure locally, possibly because my dev system is too fast. The CI systems are slower, and have slower storage. But I can guess what the problem is: the alarm_fires test disables the signal handler before dropping the Timer, and Timer::drop is what disables the timer in the OS. I'll create a PR.

@rtzoeller
Copy link
Collaborator Author

bors retry

@bors bors bot merged commit c1abab9 into nix-rust:master Jan 2, 2022
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.

2 participants