Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Testbed import #80

Merged
merged 9 commits into from
Apr 6, 2018
Merged

Conversation

carlosalberto
Copy link
Contributor

Initial import of the testbed used in other languages.

Summary:

  • It adds a new testbed directory where the tests live, with extra dependencies in setup.py (not affecting the main package, of course).
  • It includes samples for asyncio, tornado, threading and gevent, as well as basic implementation of ScopeManager for each of them.
  • A small script to run all the suite (testbed/__main__.py) checks the Python version, and disables the ones for asyncio if running under Python 2 (otherwise, doing py.test testbed/ runs all the tests). This also can be done through make testbed.
  • Included a README for the whole suite and a README per pattern, including any big semantic difference between them.

Ideally I'd like to have this merged (and adjusted if/as needed), prior to do the RC 2 ;)

@yurishkuro @palazzem @tedsuo

README.rst Outdated
^^^^^^^^^^^^^

A testbed suite designed to test API changes and experimental features is included under the `testbed` directory. For more information, see the `Testbed README <testbed/README.rst>`_.

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea why we're using .rst for readme... seems like the only OT repo that does that :-(

Copy link
Member

Choose a reason for hiding this comment

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

It's very common in Python projects (or at least it was) using .rst instead of Markdown. Some examples with widely used libraries:

I don't have strong opinions on that, since what is important to me is the content of the README and keeping it consistent across other OT repos is a good idea.

setup.py Outdated
],
'testbed': [
'six>=1.10.0,<2.0',
'futures',
Copy link
Member

Choose a reason for hiding this comment

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

I was under impression futures is not available in py3

Copy link
Member

Choose a reason for hiding this comment

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

Technically it should not be installed for py3, but since the stdlib takes precedence in modules resolution, it should not be harmful / wrong in our testbed. Conditional requires are available as follow:

setup(
    ...
    extras_require={
        ':python_version == "2.7"': ['futures']
    }
)

as described in the futures package

We don't list the actual test list anymore,
as we simply load any of them that starts with the test_
prefix.
testbed/README.rst was changed to testbed/README.md
@carlosalberto
Copy link
Contributor Author

Thanks for the feedback @yurishkuro and @palazzem

  • I updated the README (just in case) to use the .md format (I used that for the actual per-sample README anyway).
  • As @palazzem said, resolution worked just fine for futures under Python 3 (Travis didn't fail for Python 3, for example). But to be in the safe side, I made it conditional for extras_require).

Let me know ;)

@carlosalberto
Copy link
Contributor Author

Going to merge as we did for the java-examples at the time (too much to review, and we needed to get a RC by then, just as we want it for Python now ;) )

Feel free to open an issue/PR on any change regarding the testbed integration, and I will take a look. And thanks for the feedback so far!

@yurishkuro @palazzem

@carlosalberto carlosalberto merged commit 2ffae7e into opentracing:v2.0.0 Apr 6, 2018
@palazzem
Copy link
Member

palazzem commented Apr 6, 2018

Sure thing, we can iterate over the testbed as soon we discover new edge/legit use cases.

carlosalberto added a commit that referenced this pull request Jul 10, 2018
* Implement ScopeManager for in-process propagation (updated) (#64)
* Updating docstrings and tests
* Testbed import (#80)
* Scope managers integration (#83)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants