-
Notifications
You must be signed in to change notification settings - Fork 1k
Fix some non-Linux test failures #6473
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
base: main
Are you sure you want to change the base?
Conversation
The "has_gnu_date" function checks "date --help" output, but some versions of "date" don't include a "--help" flag.
`time.strftime()` has platform-specific behavior when dealing with a `gmtime()` struct.
It was removed in canonical#6356, but is still needed in some tests that interact with `/tmp`.
|
Bwahaha...CLA bites me too. I'll get that taken care of |
Many socket attributes are OS-specific, so ensure we have attributes that work for the tests that need them. Also, mock socket calls where needed.
'echo -n' isn't universal
Note that this test wasn't always Linux-specific, but when the pid stuff moved under Distro, the test was changed to test the base class (i.e., Linux) functionality only.
101edc6 to
cdbd582
Compare
|
Update: we're looking into doing the company-wide CLA, so it may be a while |
Thanks for the update. Keep us posted! |
|
Update: still waiting on CLA. Mostly just commenting to stop the stale checker from flagging this again 😄 |
|
CLA has been signed on our end, so now just waiting for whatever magic needs to happen on the Canonical side for the CLA checker to be updated. |
Woo hoo! Welcome back! |
|
@TheRealFalcon in prepartion for landing, a couple of hacktoberfest commits landed dropping tests/helpers.py::TestCase which resulted in a merge conflict. I'll make sure I touch base with Canonical CLA folks to ensure the CLA signing is permitted now for your company so we can clear the CI workflow. |
|
Noted, I'll do the rebase once the CLA checker says I'm good. I've just been manually curling it to see. |
|
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close. If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging blackboxsw, and he will ensure that someone takes a look soon. (If the pull request is closed and you would like to continue working on it, please do tag blackboxsw to reopen it.) |
holmanb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code changes look good to me. There is a merge conflict, and the CLA job hasn't passed yet, but once those are resolved I'd say that this is ready for a rebase merge.
Thanks @TheRealFalcon!
| from cloudinit import user_data as ud | ||
| from cloudinit import util | ||
| from cloudinit import ( | ||
| util, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I assume this change wasn't intended? if this is what black wants now, maybe we could just move this up with the import list above this line?
| cmd = [ | ||
| SH, | ||
| "-c", | ||
| 'echo -n "$@"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting - I didn't realize that this isn't defined in posix
|
Thanks!
Still waiting on this unfortunately. I believe we submitted it a few weeks ago. Is there something stuck on the Canonical side? |
I've copied internal folks here saying we're blocked with "external" contributors having signed the CLA. My visibility here is that legal has been asked about this directly and they CLA-folks are awaiting a response. I'll bump this topic again so we can unblock. |
Proposed Commit Message
Additional Context
With these changes (and changes to the default TMPDIR), tests pass for me on a Mac. This PR contains multiple commits and each commit should be independently reviewable. I'll leave it up to whoever merges to decide if they want to squash or not.
Test Steps
Merge type