Skip to content

Conversation

@bz2
Copy link
Contributor

@bz2 bz2 commented May 19, 2018

Resolves F841.

Also improves several tests to assert on relevant behaviours.

Copy link
Contributor

Choose a reason for hiding this comment

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

This changes the test

Copy link
Contributor Author

@bz2 bz2 May 19, 2018

Choose a reason for hiding this comment

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

It does, but the test still fails if the change from commit e5e73e5 is reversed. It's really testing the constructor not recorded values, which is covered by TestHistogram.test_histogram.

Is there a better way of asserting "this isn't a _LabelWrapper that needs labels() called before using"? I don't really want to use isinstance().

Copy link
Contributor

Choose a reason for hiding this comment

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

You could not assign to a variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I wanted a better assertion but if you're fine with just checking the registry.

Resolves F841.

Also improves several tests to assert on relevant behaviours.

Signed-off-by: Martin Packman <martin@zegami.com>
@brian-brazil brian-brazil merged commit 9551846 into prometheus:master May 21, 2018
@brian-brazil
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants