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

Move shared test code from ext/ to tests/ #559

Merged
merged 4 commits into from
Apr 15, 2020

Conversation

c24t
Copy link
Member

@c24t c24t commented Apr 6, 2020

opentelemetry-ext-testutil is a package with shared test classes used by ext packages (right now only opentelemetry-ext-flask). We don't release this package, just import it in other tests.

Right now, on each release, we build everything in ext/. This means whoever does the release has to remember to exclude this package when they push the others to PyPI.

This PR moves the files and package:

  • Move files ext/opentelemetry-ext-testutil -> tests/util
  • Move package opentelemetry.ext.testutil -> opentelemetry.test

This makes maintainers' lives easier, but it does mean that other packages that use testutils will have to install install the opentelemetry.test package from source. But this is already the case since we don't publish opentelemetry-ext-testutil.

This PR proposes that we move shared test code back into the main repo until we move a package that depends on it into a separate repo, at which point we'll have to put this code in its own top-level package.

@c24t c24t requested a review from a team April 6, 2020 22:37
@mauriciovasquezbernal
Copy link
Member

opentelemetry-ext-testutil is a package with shared test classes used by ext packages (right now only opentelemetry-ext-flask).

For the record, it's also used in opentelemetry-ext-wsgi

This PR proposes that we move shared test code back into the main repo until we move a package that depends on it into a separate repo, at which point we'll have to put this code in its own top-level package.

I think it could be better to move it to its own package right now. We are already starting to push on https://github.com/open-telemetry/opentelemetry-auto-instr-python so it could be useful there.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

This makes sense to me. I would suggest that publishing a test package would make this useful for anyone writing exporters or integrations, so it might make sense to publish it sooner than later. Though i guess anyone wanting to use this can already do so via source so it's not that urgent

@toumorokoshi toumorokoshi merged commit 7d11cf1 into open-telemetry:master Apr 15, 2020
@c24t c24t deleted the move-testutil branch April 16, 2020 19:57
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.

5 participants