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

Isolate Integration Testing Environments #1917

Open
TimPansino opened this issue May 19, 2022 · 5 comments
Open

Isolate Integration Testing Environments #1917

TimPansino opened this issue May 19, 2022 · 5 comments

Comments

@TimPansino
Copy link
Contributor

TimPansino commented May 19, 2022

This issue is a follow up / postmortem on PR #1594

Considering this PR has been unmerged and blocked by dependency hell since January, and no one has been able to properly use strawberry with recent versions of Starlette, I think you should reconsider your current CI tooling. It can be quite frustrating for contributors and users alike when issues are unfixed because of workflow issues, not because no one has tried.

I would highly suggest you use independent environments for integration testing with external libraries. It would allow you to much more easily test these without having to worry about conflicting dependencies blocking features and patches.

The other issue I see is that you now have 2 distinct versions of starlette that you could continue to support, but without regression testing older versions are more likely to break and not be noticed. “Most” package maintainers try to do matrix based testing where they test against an array of versions of other packages, and against an array of python versions as well.

To that end, I would suggest you use tox as it seems to be the tool of choice for package maintainers. It’s very simple to separate your tests into suites by folders, and have each test suite run in an individual environment with isolated dependencies. (Similar tools exist that I don’t have much experience with, see nox and thx)

As an example, we maintain a lengthy and complex tox.ini for New Relic with a long comment explaining our naming conventions at the top. There are much simpler examples in the tox docs that are enough to try the tool.

If you decide to use tox I’m happy to contribute or review designs, suggestions, and PRs.

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@patrick91
Copy link
Member

Thanks for bringing this up @TimPansino! I was trying to implement nox but I had a few issues with mypy and other dependencies, I'll take a look at thx but I think it doesn't support matrix for dependencies: https://thx.omnilib.dev/en/latest/recipes.html#version-matrix

There's also hatch which could be a good substitute to poetry, since it has more feature we need: https://hatch.pypa.io/latest/environment/#matrix though it might have the same issue as thx, I'll check 😊

I've also added matrix for starlette using GitHub actions, so I think for now we are ok. Using a tool like the ones you mentioned is the best thing, but we are not in a rush I think 😊

Considering this PR has been unmerged and blocked by dependency hell since January

By the way this is also my fault, I kept procastinating on the issue 😊

@jkimbo
Copy link
Member

jkimbo commented Oct 11, 2022

Is this still an issue?

@patrick91
Copy link
Member

@jkimbo I'd say yes, we did some isolation for Starlette and Django, but I think we might be better off by using nox/tox[1] since at the moment we depend on GitHub actions' features and I'd love to test different versions of Python (and libraries) locally too.

[1] another option is hatch but I don't if we'll migrate to it 😊

@TimPansino
Copy link
Contributor Author

@patrick91 If you're open to a PR adding tox I can write one and lay the foundation.

@TimPansino
Copy link
Contributor Author

For anyone from the future interested there is a lot of further discussion in #2247

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 a pull request may close this issue.

3 participants