Skip to content

test/util: improve run_ucmd_as_root for CICD #3518

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
Merged
sylvestre merged 8 commits into from
May 11, 2022
Merged

test/util: improve run_ucmd_as_root for CICD #3518

sylvestre merged 8 commits into from
May 11, 2022

Conversation

@jhscheer
Copy link
Contributor

@jhscheer jhscheer commented May 10, 2022 •
edited
Loading

Ensure that fn run_no_wait is only invoked if the system running the
test has sudo in $path

Previously, if run inside CICD, calling fn run_ucmd_as_root
would provoke fn run_no_wait to panic because there's no sudo.

Can be tested locally with:

cargo test test_run_ucmd_as_root --features "whoami" --no-default-features -- --show-output --nocapture

(I don't know why all the old commits popped up here !?)

@jhscheer jhscheer mentioned this pull request May 10, 2022
Merged
@sylvestre
Copy link
Contributor

sylvestre commented May 10, 2022

I guess you saw:
#3515 ;)

@jhscheer jhscheer force-pushed the run_ucmd_as_root branch from dd79569 to b7ac851 Compare May 10, 2022 14:27
@jhscheer
Copy link
Contributor Author

jhscheer commented May 10, 2022

I guess you saw: #3515 ;)

Ah okay, I'll try to investigate that cannot open: Permission denied issue.

@sylvestre
Copy link
Contributor

sylvestre commented May 10, 2022

@jhscheer Should I land #3515 now or you can fix it quickly?

@jhscheer jhscheer force-pushed the run_ucmd_as_root branch 3 times, most recently from c6e68d6 to a00d5df Compare May 10, 2022 21:16
@jhscheer
test/util: improve run_ucmd_as_root for CICD
12d7d98 
* ensure that `fn run_no_wait` is only invoked if the system running the
test has `sudo` in $path

Previously, if run inside CICD, calling `fn run_ucmd_as_root`
would provoke `fn run_no_wait` to panic because there's no `sudo`.
@jhscheer jhscheer force-pushed the run_ucmd_as_root branch from a00d5df to 12d7d98 Compare May 10, 2022 21:41
@jhscheer
Copy link
Contributor Author

jhscheer commented May 11, 2022

The checks for this PR look good now, however I had to wrap everything in if !is_ci() to get rid of this strange permission issue.

@jhscheer Should I land #3515 now or you can fix it quickly?

I think #3515 can be closed in favor of this.

@sylvestre
Copy link
Contributor

sylvestre commented May 11, 2022

So, I guess the failure


---- test_sort::test_version_empty_lines stdout ----
current_directory_resolved: 
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe sort version-empty-lines.txt -V
open: C:\Users\RUNNER~1\AppData\Local\Temp\.tmpGvU2a2\version-empty-lines.expected
current_directory_resolved: 
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe sort version-empty-lines.txt --debug -V
---- test_sort::test_version_empty_lines stderr ----
thread 'main' panicked at 'Command was expected to succeed.
stdout = 
^ no match for key
^ no match for key

^ no match for key
^ no match for key

^ no match for key
^ no match for key

^ no match for key
^ no match for key

^ no match for key
^ no match for key

^ no match for key
^ no match for key

^ no match for key
^ no match for key
1.2.3-alpha
___________
___________
1.2.3-alpha2
____________
____________
11.2.3
______
______
bar2
____
____
bar2.0.0
________
________
foo0.1
______
______
foo1.0
______
______
>>>1.12.4
_________
_________

 stderr = ', tests\common\util.rs:174:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test_sort::test_version_empty_lines

is unrelated?
thanks :)

@sylvestre
Copy link
Contributor

sylvestre commented May 11, 2022

I had to wrap everything in if !is_ci() to get rid of this strange permission issue.

I have seen some unexpected behavior too in the past

@jhscheer
Copy link
Contributor Author

jhscheer commented May 11, 2022

So, I guess the failure

failures:
    test_sort::test_version_empty_lines

is unrelated? thanks :)

Yes, that looks like an unrelated glitch.

@sylvestre
Copy link
Contributor

sylvestre commented May 11, 2022

yeah:
image

@sylvestre sylvestre merged commit 0ebd9c9 into uutils:main May 11, 2022
@sylvestre sylvestre mentioned this pull request May 11, 2022
Closed
@jfinkels
Copy link
Collaborator

jfinkels commented May 12, 2022

The documentation for GitHub Actions states that sudo is available. "When you need to execute commands or install tools that require more privileges than the current user, you can use sudo without needing to provide a password." https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#administrative-privileges-of-github-hosted-runners

So something odd is happening. Maybe PATH is not set when we spawn the process that is supposed to invoke sudo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@jhscheer @sylvestre @jfinkels