✨ Source Intercom: adding a mock server test#54715
✨ Source Intercom: adding a mock server test#54715Maxime Carbonneau-Leclerc (maxi297) merged 9 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Octavia Squidington III (octavia-squidington-iii)
added
area/connectors
| [tool.poetry.dependencies] | ||
| python = "^3.10,<3.13" | ||
| airbyte-cdk = "6.28.0" | ||
| airbyte-cdk = {path = "../../../../../airbyte-python-cdk/", develop = true} |
There was a problem hiding this comment.
To be updated once the CDK version is released
| - cdk:low-code | ||
| - language:manifest-only | ||
| connectorTestSuitesOptions: | ||
| - suite: unitTests |
There was a problem hiding this comment.
Interesting... unit tests were not executed for this connector
There was a problem hiding this comment.
Maxime Carbonneau-Leclerc (@maxi297) - Is this still an issue or did you find a solution?
There was a problem hiding this comment.
I mean, adding this line allows for the tests to run so we're good. I was just concerned that we weren't running unit tests for source-intercom
There was a problem hiding this comment.
Got it. Thanks for clarifying. This issue has gotten me before as well - specifically the omitted declaration.
Maxime Carbonneau-Leclerc (maxi297)
requested a review
from a team
as a code owner
| cd airbyte-ci/connectors/pipelines/ | ||
| poetry install | ||
| poetry shell | ||
| poetry env activate |
There was a problem hiding this comment.
It seems like this command is not available in poetry 2.X but I haven't test if the alternative mentioned here actually works
Maxime Carbonneau-Leclerc (maxi297)
requested a review
from Aaron ("AJ") Steers (aaronsteers)
There was a problem hiding this comment.
This looks good to me! Nice work unblocked the weird failure scenarios!
Long-run, I would like to have a test or tests extra so declaring the dependency on something like airbyte-cdk[tests] would also include the transitive dependencies expected for tests. But your solution here is a good one and there's no reason I can see not to merge these improvements.
Also, for future cases, we can lean into the unit_tests/pyproject.toml file declaring extra dependencies needed for tests. It's weird for it to need to declare things that are already in the CDK, but there could be other cases where it wants another library - like a mock data generator or a caching tool or S3/minio library - and those are fine to include as dependent libraries in the connnector's-own unit_tests/pyproject.toml.
Those are just thoughts for the future - I think this PR is 💎 and makes things better so let's 🚢 it when ready. 👍
Maxime Carbonneau-Leclerc (maxi297)
deleted the
maxi297/mock-server-test-for-source-intercom
branch
3 participants
What
As part of a PoC to increase test coverage and debug locally: airbytehq/airbyte-python-cdk#371
How
Update CDK version and add custom component path to sys.path
User Impact
None, we just want to have more tests in the future
Can this PR be safely reverted and rolled back?