Gate shell_completions tests on availability by ehuss · Pull Request #14541 · rust-lang/cargo (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits1 Checks21 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This changes the shell_completions tests to only run if the corresponding external program is installed. Generally, cargo's tests shouldn't require any external commands to be installed.
This also uses the ignore attribute for disabling on macos. It is quite confusing when running on macos to see a successful test when it isn't running. This ensures that the test output indicates that it is ignored and the reason.
cc @shannmu
r? @weihanglo
rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Hm, so this approach isn't going to work. It's too late for me to continue looking at this, will maybe look at it tomorrow or the next few days.
error: failed to install component: 'llvm-tools-preview-x86_64-unknown-linux-gnu', detected conflict: 'lib/rustlib/x86_64-unknown-linux-gnu/bin/llc'
@bors try
bors added a commit that referenced this pull request
Gate shell_completions tests on availability
This changes the shell_completions tests to only run if the corresponding external program is installed. Generally, cargo's tests shouldn't require any external commands to be installed.
This also uses the ignore attribute for disabling on macos. It is quite confusing when running on macos to see a successful test when it isn't running. This ensures that the test output indicates that it is ignored and the reason.
cc @shannmu
Status: The marked PR is awaiting some action (such as code changes) from the PR author.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
I recommend we be free with reverts when a code change is blocking. We can always fix this in another pass.
📌 Commit fdf4f26 has been approved by weihanglo
It is now in the queue for this repository.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-author
Status: The marked PR is awaiting some action (such as code changes) from the PR author.
labels
bors added a commit that referenced this pull request
Gate shell_completions tests on availability
This changes the shell_completions tests to only run if the corresponding external program is installed. Generally, cargo's tests shouldn't require any external commands to be installed.
This also uses the ignore attribute for disabling on macos. It is quite confusing when running on macos to see a successful test when it isn't running. This ensures that the test output indicates that it is ignored and the reason.
cc @shannmu
@bors r-
This isn't going to work, since not all jobs have the shells installed.
I'm looking at a different approach where the jobs will be identified as being "extended" to test these extra things that might not be available.
Status: The marked PR is awaiting some action (such as code changes) from the PR author.
and removed S-waiting-on-bors
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
labels
And BTW, this will block updating in rust-lang/rust, since those won't have the shells installed either. I'm poking around now, but I'm a little confused why the fish test seems to be randomly failing.
The Fish shell test failures might be due to the test depending on the completest crate, which tests the completion results based on an interactive shell. We cannot be certain when the completion results are fully generated, so there is a built-in timeout parameter. If the completion results are not generated within the timeout period, it may cause the test to fail.
Hmm. That's unfortunate.
Let's just disable the tests for now. I have opened #14546 to do that, and #14545 lists the problems that I noticed while looking at this.
I'm probably not going to look at this further for now.
Labels
Status: The marked PR is awaiting some action (such as code changes) from the PR author.