Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add CI, make tests pass on non-linux#86

Merged
brian-brazil merged 1 commit intoprometheus:masterprometheus/client_python:masterfrom
hynek:masterCopy head branch name to clipboard
Jul 8, 2016
Merged

Add CI, make tests pass on non-linux#86
brian-brazil merged 1 commit intoprometheus:masterprometheus/client_python:masterfrom
hynek:masterCopy head branch name to clipboard

Conversation

@hynek
Copy link
Contributor

@hynek hynek commented Jun 17, 2016

This is mainly to remove friction from contributing.

A couple of notes:

  • running tests over all python versions is now a matter of running tox
  • you’ll probably have to activate the project in Travis
  • it doesn’t run tests on 2.6 because…the tests currently fail on it.
  • it uses py.test to have a consistent test runner. It doesn’t use any of its features tho. Test discovery via python -m unittest works differently across python versions. I could also use nose or something but I chose pytest as it’s more familiar to me.
  • The odd-at-first-look coverage construction allows for measuring coverage over multiple Python versions.
  • I’d recommend to add some more targets but let’s start simple.

Let me know if you have any questions or concerns.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's only OS X that's broken.

@brian-brazil
Copy link
Contributor

it doesn’t run tests on 2.6 because…the tests currently fail on it.

I'd like to test 2.6. That looks to be a mix of a dependency issue, and not having the improved float printing semantics for histogram buckets. Could the first be fixed, and the second excluded from testing?

@hynek
Copy link
Contributor Author

hynek commented Jun 23, 2016

While I think that 2.6 is terrible and nobody should use it, I would suggest to put it in a separate PR.

@hynek
Copy link
Contributor Author

hynek commented Jun 25, 2016

r4r btw

@brian-brazil
Copy link
Contributor

CentOS 6 is still supported and provides python 2.6, we have to work with what our users have not what we wish they have.

I'd not have separate tests for twisted, there's be various other optional deps over time so that won't scale well.

@hynek
Copy link
Contributor Author

hynek commented Jul 4, 2016

I’m not sure how your comment relates to my suggestion to put it into a separate PR?

@hynek
Copy link
Contributor Author

hynek commented Jul 4, 2016

As for the second part, you want to have code paths that are not covered by CI on purpose? How do you want to ensure long-term that stuff doesn’t break?

@brian-brazil
Copy link
Contributor

The code paths would only be not covered on Python 2.6, the chances of them breaking only there is pretty low as that particular piece of code is stable.

@hynek
Copy link
Contributor Author

hynek commented Jul 6, 2016

Your answer kind of mixes my two questions. :)

Let me try again:

  1. do you insist on resolving Python 2.6 within this PR or is it OK to put it in a separate one?
  2. do you really want to keep the Twisted code from CI?

@brian-brazil
Copy link
Contributor

It's ok to have in a separate one.

I'm thinking more that a separate run just for twisted might be a bit much, and it should be combined into the main one.

@hynek
Copy link
Contributor Author

hynek commented Jul 7, 2016

Alright!

So I would suggest the following: let twisted be part of regular run but have one py35-notwisted that ensures, that prometheus_client keeps working if Twisted is not installed.

Simply because it's really easy to screw up by importing something somewhere with unintended consequences.

@hynek
Copy link
Contributor Author

hynek commented Jul 7, 2016

Since your main concern was, that the list of optional deps is gonna grow, I ended up calling the env py35-nooptionals to give it room for the future.

Let me know what you think.

@brian-brazil
Copy link
Contributor

That looks generally fine, can you add a nooptions on 2.7 too so both major versions are covered?

@hynek
Copy link
Contributor Author

hynek commented Jul 8, 2016

sure; done.

@brian-brazil
Copy link
Contributor

Can you squash your commits please?

@hynek
Copy link
Contributor Author

hynek commented Jul 8, 2016

Done.

I’ve activated Travis for my fork and you can see the latest build here btw: https://travis-ci.org/hynek/client_python/builds/143289122

@brian-brazil brian-brazil merged commit ab5b401 into prometheus:master Jul 8, 2016
@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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.