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

[rabbitmq] Improve test suite and fix tagging #1472

Merged
merged 2 commits into from
Mar 31, 2015
Merged

Conversation

LeoCavaille
Copy link
Member

  • because we checked the tags only to be non-None we were creating
    empty tags like rabbitmq_policy:
  • adds some helper methods in AgentCheckTest to assert directly a
    service check by status like assertServiceCheckCritical without
    having to import AgentCheck just for that in the test file
  • add coverage report to ensure we cover 100% of the metrics output
    add TODO instead of commenting the missing metrics, maybe we can do
    that at some point but we would have to create a consumer and do some
    work to get these. For instance use the pika python module.
  • Bonus: some pep8-ing

* because we checked the tags only to be non-`None` we were creating
  empty tags like `rabbitmq_policy:`
* adds some helper methods in `AgentCheckTest` to assert directly a
  service check by status like `assertServiceCheckCritical` without
  having to import `AgentCheck` just for that in the test file
* add coverage report to ensure we cover 100% of the metrics output
  add TODO instead of commenting the missing metrics, maybe we can do
  that at some point but we would have to create a consumer and do some
  work to get these. For instance use the `pika` python module.
* Bonus: some pep8-ing
@LeoCavaille LeoCavaille added this to the 5.3.0 milestone Mar 25, 2015

for data_line in data[:max_detailed]:
# We truncate the list of nodes/queues if it's above the limit
self._get_metrics(data_line, object_type)


def _get_metrics(self, data, object_type):
tags = []
tag_list = TAGS_MAP[object_type]
for t in tag_list.keys():
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it's not your code but we shouldn't use

.keys()

here

@LeoCavaille
Copy link
Member Author

Thanks @remh for catching that, will merge when tests have passed (apache being a known failure)

LeoCavaille added a commit that referenced this pull request Mar 31, 2015
[rabbitmq] Improve test suite and fix tagging
@LeoCavaille LeoCavaille merged commit f27b15d into master Mar 31, 2015
@LeoCavaille LeoCavaille deleted the leo/rabbitmqtest branch March 31, 2015 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants