-
Notifications
You must be signed in to change notification settings - Fork 4
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
Test foreman #18
base: master
Are you sure you want to change the base?
Test foreman #18
Conversation
Added a ping test as well that replaces https://github.com/theforeman/forklift/blob/master/bats/fb-test-katello.bats |
tests/test_foreman.py
Outdated
def test_foreman_version(api, variables): | ||
if 'foreman_version' not in variables: | ||
pytest.skip("'foreman_version' is not set") | ||
|
||
status = api.resource('home').call('status') | ||
|
||
assert status['version'] == variables['foreman_version'] |
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.
This is not entirely the same as the previous check. That asserted that foreman, foreman-proxy and foreman-installer all had the same version (maybe even foreman-selinux).
Perhaps we should do both? Can we find out the Smart Proxy version via the API?
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 UI shows the smart proxy version, I am not seeing that in the API. I can look into trying to test this via testinfra
that will just require that smoker has SSH access to the server. Or we make an API update to Foreman to report the versions of these particular projects.
I think this particular test is predominantly used just to make this check for release purposes. Perhaps it belongs more so in the CI than in this test suite?
def test_check_smart_proxy_registered(api, base_url): | ||
hostname = urlparse(base_url).hostname | ||
assert api.resource('smart_proxies').call('show', {'id': hostname}) |
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.
This asserts that there is a Smart Proxy on the same host as Foreman. However, that is not always true. For example, on my own Foreman server there is just Foreman, no Smart Proxy. That's on a separate server. We should not bake in this assumption that isn't true.
I think expected proxies as a variable is a better check. That also allows asserting that there is an external Smart Proxy like we have in the Katello test setup.
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 I can make that improvement.
Needs a rebase now. |
8b70e38
to
df32fc5
Compare
Additionally replaces fb-test-katello.bats
Builds on the client tests PR and aims to replace https://github.com/theforeman/forklift/blob/master/bats/fb-test-foreman.bats