-
Notifications
You must be signed in to change notification settings - Fork 36
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
Fix proxy-auth and proxy-kickstart tests #1184
Conversation
@rvykydal FYI I hope the Travis CI failure is not caused by the tests - they pass just fine for me locally (executed via |
/test-os-variants |
proxy-auth.ks.in
Outdated
os_version=$(. /etc/os-release && echo ${VERSION_ID%%.*}) | ||
platform=$(. /etc/os-release && echo ${ID}) | ||
|
||
if [ "${platform}" == "rhel" ] && [ ${os_version} -lt 10 ]; then |
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.
Regarding the per-platform conditions, there is an open PR #1174 which adds get_platform() function that could be used similarly as in repo-install test when the PR is merged. I hope we merge it soon so you can use it. Or I can take care of it in a separate PR.
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 #1174 is merged.
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.
Thanks for the suggestion! In the end I realized I can actually use the substituted values of OS name and version (I'd have to have them separated in the condition expressions anyway), so I used this approach instead of get_platform()
- I hope it's OK.
/test-os-variants |
1 similar comment
/test-os-variants |
proxy-auth.sh
Outdated
@@ -29,6 +29,9 @@ prepare() { | |||
local tmp_dir=$2 | |||
local httpd_url="" | |||
local proxy_url="" | |||
httpd_url=$(cat ${tmpdir}/httpd_url) | |||
proxy_url=$(cat ${tmpdir}/proxy_url) | |||
local proxy_rc |
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.
I think proxy_rc is not used and we can remove it.
proxy-kickstart.sh
Outdated
@@ -30,6 +30,9 @@ prepare() { | |||
local tmp_dir=$2 | |||
local httpd_url="" | |||
local proxy_url="" | |||
httpd_url=$(cat ${tmpdir}/httpd_url) | |||
proxy_url=$(cat ${tmpdir}/proxy_url) | |||
local proxy_rc |
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.
I think proxy_rc is not used and we can remove it.
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.
Looks good to me, thank you!
7e34802
to
ffb6dfa
Compare
This covers issues rhinstaller#680 and rhinstaller#1108.
ffb6dfa
to
548913b
Compare
/test-os-variants |
This covers issue #1108.
The change of dnf command/options order is necessary to work around https://bugzilla.redhat.com/show_bug.cgi?id=2280877 (DNF5 seems to have a bit of inconsistency in how it handles the options order, at least in some cases, compared to DNF4).