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

Fix ui/io-checks/inaccessbile-temp-dir.rs test #110801

Merged
merged 4 commits into from
Apr 28, 2023
Merged

Fix ui/io-checks/inaccessbile-temp-dir.rs test #110801

merged 4 commits into from
Apr 28, 2023

Conversation

WaffleLapkin
Copy link
Member

Fixes #110794

r? @jyn514

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 25, 2023
@WaffleLapkin
Copy link
Member Author

It's worth noting that a similar non-ice-error-on-worker-io-fail.rs is actually fine as is, since -o doesn't use create_dir_all or alike and actually always fails on /does-not-exist/output (although now that I said it... do we need to change this to a local path?...)

@jyn514
Copy link
Member

jyn514 commented Apr 25, 2023

It's worth noting that a similar non-ice-error-on-worker-io-fail.rs is actually fine as is, since -o doesn't use create_dir_all or alike and actually always fails on /does-not-exist/output (although now that I said it... do we need to change this to a local path?...)

I think this would be nice but not necessary, since it's not failing spuriously anywhere.

jyn514
jyn514 approved these changes Apr 25, 2023
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Thanks! r=me with the nit fixed

tests/run-make/inaccessbile-temp-dir/Makefile Outdated Show resolved Hide resolved
@WaffleLapkin
Copy link
Member Author

@jyn514 would -o ./does-not-exist/output work, or would this require something else?

@jyn514
Copy link
Member

jyn514 commented Apr 25, 2023

@WaffleLapkin that would work for a UI test I think, yes. not 100% sure without trying it.

@WaffleLapkin
Improve non-ice-error-on-worker-io-fail.rs test
e6ed0ca 
- Remove unrelated code
- Use a local dir path
@jyn514
Copy link
Member

jyn514 commented Apr 25, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Apr 25, 2023

📌 Commit e6ed0ca has been approved by jyn514

It is now in the queue for this repository.

@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 Apr 25, 2023
@matthiaskrgr matthiaskrgr mentioned this pull request Apr 25, 2023
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 25, 2023
@matthiaskrgr
Rollup merge of rust-lang#110801 - WaffleLapkin:io-tests, r=jyn514
abaa724 
Fix `ui/io-checks/inaccessbile-temp-dir.rs` test

Fixes rust-lang#110794

r? `@jyn514`
@matthiaskrgr
Copy link
Member

@bors r-
#110818 (comment)

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 25, 2023
@WaffleLapkin
Copy link
Member Author

@bors r=jyn514 rollup=never

I suspect there might be more CI issues so let's not rollup

@bors
Copy link
Contributor

bors commented Apr 27, 2023

📌 Commit 1d847b8 has been approved by jyn514

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 27, 2023
@bors
Copy link
Contributor

bors commented Apr 28, 2023

⌛ Testing commit 1d847b8 with merge 9a3258f...

@bors
Copy link
Contributor

bors commented Apr 28, 2023

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 9a3258f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 28, 2023
@bors bors merged commit 9a3258f into rust-lang:master Apr 28, 2023
@rustbot rustbot added this to the 1.71.0 milestone Apr 28, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9a3258f): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -2.4% [-2.4%, -2.4%] 1
All ❌✅ (primary) - - 0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 2.8% [2.5%, 3.3%] 5
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) - - 0

@WaffleLapkin WaffleLapkin deleted the io-tests branch April 28, 2023 11:11
@jyn514 jyn514 mentioned this pull request Apr 28, 2023
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 28, 2023
@matthiaskrgr
Rollup merge of rust-lang#110948 - TimNN:patch-1, r=WaffleLapkin
c8430b2 
run-make test: using single quotes to not trigger the shell

This test got added in rust-lang#110801.

I'm no expert on Makefiles, but IIUC this command is passed to the shell, which usually tries to execute commands specified in between backticks in double-quoted strings.

Using single quotes should fix this, I think. (Note: Waiting for CI to test this, since I only have a web browser available right now).

r? `@jyn514`

cc `@WaffleLapkin`

Since this is breaking our build bot, even if it is not directly LLVM related: `@rustbot` label: +llvm-main
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 28, 2023
@matthiaskrgr
Rollup merge of rust-lang#110948 - TimNN:patch-1, r=WaffleLapkin
afbb188 
run-make test: using single quotes to not trigger the shell

This test got added in rust-lang#110801.

I'm no expert on Makefiles, but IIUC this command is passed to the shell, which usually tries to execute commands specified in between backticks in double-quoted strings.

Using single quotes should fix this, I think. (Note: Waiting for CI to test this, since I only have a web browser available right now).

r? ``@jyn514``

cc ``@WaffleLapkin``

Since this is breaking our build bot, even if it is not directly LLVM related: ``@rustbot`` label: +llvm-main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 approved these changes

Assignees

@jyn514 jyn514

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

tests/ui/io-checks fail locally on master
6 participants
@WaffleLapkin @jyn514 @bors @matthiaskrgr @rust-timer @rustbot