-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Task/increase test coverage #120
Conversation
Are we abiding by the CircleCI testing now? |
@jmeixensperger yes we consider the CI results gating. There's a few test failures, waiting on some splunk-ansible changes to make it into develop. |
# Poll for the container to be ready | ||
assert self.wait_for_containers(1, name=splunk_container_name) | ||
# Check splunkd | ||
splunkd_port = self.client.port(cid, 8089)[0]["HostPort"] |
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.
It seems like theres a bit of code duplication on this segment, where you verify splunk is up and responding at the /services/server/info path. Would this be better as its own method, to avoid the duplication?
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.
+1, there is already a method that does this but I don't think I call it here and probably should. I'll fix this in my next PR when I add more tests.
No description provided.