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

Make the kernel_copy tests more robust/concurrent. #79375

Merged
merged 1 commit into from
Dec 11, 2020

Conversation

vext01
Copy link
Contributor

@vext01 vext01 commented Nov 24, 2020

These tests write to the same filenames in /tmp and in some cases these files don't get cleaned up properly. This caused issues for us when different users run the tests on the same system, e.g.:

---- sys::unix::kernel_copy::tests::bench_file_to_file_copy stdout ----
thread 'sys::unix::kernel_copy::tests::bench_file_to_file_copy' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', library/std/src/sys/unix/kernel_copy/tests.rs:71:10
---- sys::unix::kernel_copy::tests::bench_file_to_socket_copy stdout ----
thread 'sys::unix::kernel_copy::tests::bench_file_to_socket_copy' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', library/std/src/sys/unix/kernel_copy/tests.rs💯10

Use std::sys_common::io__test::tmpdir() to solve this.

CC @the8472.

@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 24, 2020
@vext01
Copy link
Contributor Author

vext01 commented Nov 24, 2020

There's a typo in the commit message: issue -> issues. I can fix that just before bors.

@the8472
Copy link
Member

the8472 commented Nov 24, 2020

LGTM

@vext01
Copy link
Contributor Author

vext01 commented Nov 24, 2020

Can I force push to fix the typo?

@the8472
Copy link
Member

the8472 commented Nov 24, 2020

There's no policy against that. CI will check your commits again anway.

@vext01
Copy link
Contributor Author

vext01 commented Nov 24, 2020

Fixed.

@vext01
Copy link
Contributor Author

vext01 commented Nov 26, 2020

I guess @joshtriplett is busy. Is someone else able to start the merge? @the8472?

@the8472
Copy link
Member

the8472 commented Nov 26, 2020

I don't have the necessary permissions to do that. You'll have to wait. IME PRs may take a few weeks, depending on how busy team members are.

@vext01
Copy link
Contributor Author

vext01 commented Dec 2, 2020

Is anyone able to kick the merge off for this? @joshtriplett ?

Sorry to be so persistent, but this bug is frequently biting us in our CI pipeline :(

@the8472
Copy link
Member

the8472 commented Dec 2, 2020

In the meantime you might want to update the PR to also apply the tempdir changes to the test added by #79600

These tests write to the same filenames in /tmp and in some cases these
files don't get cleaned up properly. This caused issues for us when
different users run the tests on the same system, e.g.:

```
---- sys::unix::kernel_copy::tests::bench_file_to_file_copy stdout ----
thread 'sys::unix::kernel_copy::tests::bench_file_to_file_copy' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', library/std/src/sys/unix/kernel_copy/tests.rs:71:10
---- sys::unix::kernel_copy::tests::bench_file_to_socket_copy stdout ----
thread 'sys::unix::kernel_copy::tests::bench_file_to_socket_copy' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', library/std/src/sys/unix/kernel_copy/tests.rs:100:10
```

Use `std::sys_common::io__test::tmpdir()` to solve this.
@vext01
Copy link
Contributor Author

vext01 commented Dec 3, 2020

done!

@bjorn3
Copy link
Member

bjorn3 commented Dec 10, 2020

@bors r+ rollup

I didn't previously approve it as I wasn't sure if this PR was in scope of the r+ rights I recently got, but I just got a confirmation that it is fine.

@bors
Copy link
Contributor

bors commented Dec 10, 2020

📌 Commit 87c1fdb has been approved by bjorn3

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 10, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 10, 2020
Make the kernel_copy tests more robust/concurrent.

These tests write to the same filenames in /tmp and in some cases these files don't get cleaned up properly. This caused issues for us when different users run the tests on the same system, e.g.:

```
---- sys::unix::kernel_copy::tests::bench_file_to_file_copy stdout ----
thread 'sys::unix::kernel_copy::tests::bench_file_to_file_copy' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', library/std/src/sys/unix/kernel_copy/tests.rs:71:10
---- sys::unix::kernel_copy::tests::bench_file_to_socket_copy stdout ----
thread 'sys::unix::kernel_copy::tests::bench_file_to_socket_copy' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 13, kind: PermissionDenied, message: "Permission denied" }', library/std/src/sys/unix/kernel_copy/tests.rs💯10
```

Use `std::sys_common::io__test::tmpdir()` to solve this.

CC `@the8472.`
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 11, 2020
Rollup of 11 pull requests

Successful merges:

 - rust-lang#77027 (Improve documentation for `std::{f32,f64}::mul_add`)
 - rust-lang#79375 (Make the kernel_copy tests more robust/concurrent.)
 - rust-lang#79639 (Add long explanation for E0212)
 - rust-lang#79698 (Add tracking issue template for library features.)
 - rust-lang#79809 (Dogfood `str_split_once()`)
 - rust-lang#79851 (Clarify the 'default is only allowed on...' error)
 - rust-lang#79858 (Update const-fn doc in unstable-book)
 - rust-lang#79860 (Clarify that String::split_at takes a byte index.)
 - rust-lang#79871 (Fix small typo in `wrapping_shl` documentation)
 - rust-lang#79896 (Make search results tab and help button focusable with keyboard)
 - rust-lang#79917 (Use Symbol for inline asm register class names)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a8c19e1 into rust-lang:master Dec 11, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 11, 2020
@vext01 vext01 deleted the kernel-copy-temps branch December 11, 2020 10:21
vext01 added a commit to vext01/ykrustc that referenced this pull request Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants