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

Skip tests running locally without root access #16222

Merged

Conversation

farbodahm
Copy link
Contributor

@farbodahm farbodahm commented Feb 18, 2023

After a short discussion with @tgross, I learned that some of the tests will fail if we run them locally and without root access.

This PR will add a functionality to skip tests if the tests:

  • Are not running in CI environment (they are running locally)
  • and if they are not running with Root access

Also it will log that the test is skipped, so if user wants to run them, she/he has to run it with root access.

In the current behavior, They are failing by saying something like: operation not permitted.

Note: I used make test | grep -C 5 "operation not permitted" to find which tests require Root access, But it may not contain all of the tests which require root access.

Please let me know if anything is needed; Thanks.

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Hi @farbodahm! The CI run 7312099268 is showing a test panic in TestGenericNotifier in helper/broker/notify_test.go that you'll want to look into.

@farbodahm
Copy link
Contributor Author

@tgross I double checked that the part which I changed didn't have any effect on the test which was failing. Also I ran the test with different parameters locally and it was fine; So I just re-run the CI (by force pushing) and everything is fine now.

@tgross
Copy link
Member

tgross commented Feb 23, 2023

Hey @farbodahm I'm tied up in another mini-project I'm trying to ship by end of the week. I'll pick this back up next week. Just wanted to let you know that I haven't forgotten about this! 😀

@farbodahm
Copy link
Contributor Author

@tgross No worries man, Please take your time and thanks for mentioning!

@tgross tgross self-assigned this Feb 24, 2023
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

Hi @farbodahm! This looks pretty good once the changes I've posted are resolved.

ci/skip_non_root.go Outdated Show resolved Hide resolved
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM!

I'll have to wait a day to merge this -- we're in a very brief merge freeze on main because 1.5.0 GA is coming out shortly. But once that's live I can get this in. Thanks again, @farbodahm!

@farbodahm
Copy link
Contributor Author

@tgross No worries, Thank you so much for the review Tim.

@tgross tgross merged commit fbd0dcb into hashicorp:main Mar 2, 2023
philrenaud pushed a commit that referenced this pull request Mar 14, 2023
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