feat(airbyte-ci): Enable manifest-only connector unit tests in CI #48790

ChristoGrab · 2024-12-03T23:48:03Z

What

Enables unit testing capabilities for manifest-only connectors and validates the approach using source-the-guardian-api as a proof of concept. This change allows manifest-only connectors to maintain test coverage of custom components while benefiting from the simplified manifest-based architecture.

Demo: https://www.loom.com/share/80b3ae7282e94dd0acfa34854b877462?sid=18b54fd6-79e0-4154-83ba-462d4345ddcb

Note

The changes in this PR are reliant on the availability of the test fixtures in the CDK. The PR to include those fixtures can be found here: airbytehq/airbyte-python-cdk#121

How

Guardian API PoC:

Implemented unit tests for the source's CustomPageIncrement component
Set the base_image version to a release candidate of the CDK containing the fixtures needed to dynamically load components.py

airbyte-ci

Modified airbyte-ci to support running unit tests for manifest-only connectors:
Added ManifestConnectorUnitTests class inheriting from PytestStep
Enabled unit test execution during CI

This approach provides a bridge for maintaining test coverage as we transition connectors to the manifest-only format, ensuring we can continue to maintain existing unit tests for custom Python components.

How to Test

Pull this branch locally and navigate to the airbyte-integrations/connectors/source-the-guardian-api/unit_tests folder
Run poetry install and poetry run pytest to verify the tests run locally.
Navigate to airbyte-ci/connectors/pipelines and run poetry install and poetry shell
Run airbyte-ci connectors --name=source-the-guardian-api test --only-step=unit to run just the unit test suite against a built connector image.
The results of the full test run can also be verified in the CI checks for this PR.

Can this PR be safely reverted and rolled back?

YES 💚
NO ❌

…ectors

vercel · 2024-12-03T23:48:07Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Dec 12, 2024 0:21am

airbyte-integrations/connectors/source-the-guardian-api/unit_tests/conftest.py

.../connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/manifest_only_connectors.py

ChristoGrab · 2024-12-05T01:08:56Z

airbyte-integrations/connectors/source-the-guardian-api/unit_tests/pyproject.toml

+[tool.poetry.dependencies]
+python = "^3.10"
+airbyte-cdk = "5.17.0"
+pytest = "^8"


Two things to note:

I opted out of dev dependencies as the whole project is just a testing harness and I don't believe there is any need for separation of main/dev deps.

For this iteration, the CDK version will have to be manually matched to whatever is listed in the connector's metadata. It should be a small pull to add some logic to up-to-date that automatically does this for us.

ChristoGrab · 2024-12-05T01:14:11Z

airbyte-integrations/connectors/source-the-guardian-api/unit_tests/test_paginator.py

+def test_page_increment(components_module, current_page, total_pages, expected_next_page):
+    """Test the CustomPageIncrement pagination for various page combinations"""
+
+    CustomPageIncrement = components_module.CustomPageIncrement


Just highlighting that this is how we can reference individual components within test files

bnchrch

Great work. Just had a few questions. Nothing that should be blocking though

.../connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/manifest_only_connectors.py

airbyte-integrations/connectors/source-the-guardian-api/unit_tests/conftest.py

…t statements

ChristoGrab · 2024-12-09T23:51:08Z

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (77ef2a4)

bnchrch · 2024-12-10T19:30:18Z

airbyte-integrations/connectors/source-the-guardian-api/unit_tests/test_paginator.py

+    ],
+    ids=["First page", "Middle page", "Penultimate page", "Last page", "Single page"]
+)
+def test_page_increment(connector_dir, components_module, current_page, total_pages, expected_next_page):


@ChristoGrab I see something sus.

Since you import components_module at the top and reference it as an argument.

Which is it using?

What happens if you remove the argument version?

Suggested change

def test_page_increment(connector_dir, components_module, current_page, total_pages, expected_next_page):

def test_page_increment(connector_dir, current_page, total_pages, expected_next_page):

Switched to using the plugin approach and it works without the import statement 🎉

bnchrch · 2024-12-10T19:31:59Z

airbyte-integrations/connectors/source-the-guardian-api/unit_tests/test_paginator.py

+import airbyte_cdk
+import pytest
+import requests
+from airbyte_cdk.test.utils.manifest_only_fixtures import components_module, connector_dir


❓ Why do we have to import?

Should this not be a pytest plugin that adds the fixture as an option for any test?

e.g.

# conftest.py pytest_plugins = ["airbyte_cdk.test_fixtures"]

# some_unit_test.py def test_ connector_dir(connector_dir): assert connector_dir.exists()

Yup, this approach seems to work and allows us to remove the import 👍 Should see the results in CI soon

ChristoGrab · 2024-12-10T22:01:14Z

/format-fix

Format-fix job started... Check job output.

✅ Changes applied successfully. (9098198)

bnchrch

Well done, well implemented

sentry-io · 2024-12-12T20:18:54Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ FileNotFoundError: Could not find pyproject.toml in the unit_tests directory for source-trello. pipelines.airbyte_ci.connectors.test.steps.mani... View Issue

_{Did you find this useful? React with a 👍 or 👎}

Co-authored-by: Octavia Squidington III <[email protected]>

ChristoGrab added 2 commits December 3, 2024 13:14

feat: add unit test harness to source-the-guardian-api

b7aef73

feat: add unit test step to airbyte-ci command for manifest-only conn…

1c52847

…ectors

octavia-squidington-iii added area/connectors Connector related issues connectors/source/the-guardian-api labels Dec 3, 2024

chore: format/version-bumps/changelogs

6e134d6

octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Dec 4, 2024

vercel bot had a problem deploying to Preview December 4, 2024 22:39 Failure

chore: simplify pyproject

8c74562

ChristoGrab commented Dec 5, 2024

View reviewed changes

airbyte-integrations/connectors/source-the-guardian-api/unit_tests/conftest.py Show resolved Hide resolved

ChristoGrab requested review from bnchrch and natikgadzhi December 5, 2024 00:15

ChristoGrab marked this pull request as ready for review December 5, 2024 00:16

ChristoGrab requested a review from a team December 5, 2024 00:16

octavia-squidington-iv requested a review from a team December 5, 2024 00:17

ChristoGrab commented Dec 5, 2024

View reviewed changes

.../connectors/pipelines/pipelines/airbyte_ci/connectors/test/steps/manifest_only_connectors.py Outdated Show resolved Hide resolved

ChristoGrab commented Dec 5, 2024

View reviewed changes

bnchrch reviewed Dec 5, 2024

View reviewed changes

octavia-squidington-iv requested a review from a team December 5, 2024 15:14

ChristoGrab added 10 commits December 5, 2024 12:40

refactor: point to CDK fixtures and update poetry config

705258a

chore: update base image and format

7d2c1e1

chore: remove useless params override

7b8a107

testing

4f71645

chore: merge master

f5daea1

feat: working ci test flow

0a4b409

chore: format

e185fef

fix: module error

fae0ad9

fix: working pipeline

714a329

refactor: simplify test steps, fix unit test assertion, clean up prin…

0612e08

…t statements

Merge branch 'master' into christo/sdm-unit-test-poc

36aadcf

vercel bot deployed to Preview December 9, 2024 23:47 View deployment

chore: auto-fix lint and format issues

77ef2a4

ChristoGrab requested a review from bnchrch December 10, 2024 00:37

Merge branch 'master' into christo/sdm-unit-test-poc

f944a05

vercel bot deployed to Preview December 10, 2024 18:58 View deployment

bnchrch reviewed Dec 10, 2024

View reviewed changes

Merge branch 'master' into christo/sdm-unit-test-poc

be242e2

octavia-squidington-iv requested a review from a team December 10, 2024 21:11

vercel bot deployed to Preview December 10, 2024 21:16 View deployment

refactor: use plugin over importing

ebd123b

chore: auto-fix lint and format issues

9098198

ChristoGrab requested a review from bnchrch December 10, 2024 22:28

bnchrch approved these changes Dec 10, 2024

View reviewed changes

ChristoGrab and others added 2 commits December 11, 2024 13:47

chore: bump to stable CDK/SDM release

a9e8371

Merge branch 'master' into christo/sdm-unit-test-poc

7980b3e

vercel bot deployed to Preview December 11, 2024 21:58 View deployment

fix: specify root user when installing test dependencies

a22ae6b

ChristoGrab enabled auto-merge (squash) December 12, 2024 00:41

ChristoGrab changed the title ~~PoC: Enable manifest-only connector unit tests~~ feat(airbyte-ci): Enable manifest-only connector unit tests in CI Dec 12, 2024

ChristoGrab merged commit 3b3c965 into master Dec 12, 2024
38 checks passed

ChristoGrab deleted the christo/sdm-unit-test-poc branch December 12, 2024 00:43

barduinor pushed a commit to box-community/airbyte that referenced this pull request Dec 17, 2024

PoC: Enable manifest-only connector unit tests (airbytehq#48790)

6c0789b

Co-authored-by: Octavia Squidington III <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(airbyte-ci): Enable manifest-only connector unit tests in CI #48790

feat(airbyte-ci): Enable manifest-only connector unit tests in CI #48790

ChristoGrab commented Dec 3, 2024 •

edited

Loading

vercel bot commented Dec 3, 2024 •

edited

Loading

ChristoGrab Dec 5, 2024 •

edited

Loading

ChristoGrab Dec 5, 2024 •

edited

Loading

bnchrch left a comment

ChristoGrab commented Dec 9, 2024 •

edited by github-actions bot

Loading

bnchrch Dec 10, 2024

ChristoGrab Dec 10, 2024

bnchrch Dec 10, 2024

ChristoGrab Dec 10, 2024 •

edited

Loading

ChristoGrab commented Dec 10, 2024 •

edited by github-actions bot

Loading

bnchrch left a comment

sentry-io bot commented Dec 12, 2024

	def test_page_increment(connector_dir, components_module, current_page, total_pages, expected_next_page):
	def test_page_increment(connector_dir, current_page, total_pages, expected_next_page):

feat(airbyte-ci): Enable manifest-only connector unit tests in CI #48790

feat(airbyte-ci): Enable manifest-only connector unit tests in CI #48790

Conversation

ChristoGrab commented Dec 3, 2024 • edited Loading

What

Note

How

Guardian API PoC:

airbyte-ci

How to Test

Can this PR be safely reverted and rolled back?

vercel bot commented Dec 3, 2024 • edited Loading

ChristoGrab Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

ChristoGrab Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

bnchrch left a comment

Choose a reason for hiding this comment

ChristoGrab commented Dec 9, 2024 • edited by github-actions bot Loading

bnchrch Dec 10, 2024

Choose a reason for hiding this comment

ChristoGrab Dec 10, 2024

Choose a reason for hiding this comment

bnchrch Dec 10, 2024

Choose a reason for hiding this comment

ChristoGrab Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

ChristoGrab commented Dec 10, 2024 • edited by github-actions bot Loading

bnchrch left a comment

Choose a reason for hiding this comment

sentry-io bot commented Dec 12, 2024

Suspect Issues

ChristoGrab commented Dec 3, 2024 •

edited

Loading

vercel bot commented Dec 3, 2024 •

edited

Loading

ChristoGrab Dec 5, 2024 •

edited

Loading

ChristoGrab Dec 5, 2024 •

edited

Loading

ChristoGrab commented Dec 9, 2024 •

edited by github-actions bot

Loading

ChristoGrab Dec 10, 2024 •

edited

Loading

ChristoGrab commented Dec 10, 2024 •

edited by github-actions bot

Loading