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

[airbyte-ci] test connectors inside their built container #30474

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Sep 15, 2023

What

Our original implementation of python connector testing was the following:

  • Create a python container
  • Mount the connector code to it
  • Install some testing dependencies
  • Run pytest

The problem with this approach is that we're running connector tests inside their real environment: their docker image.
So far our test environment had to keep up with the system dependencies of the connectors.
These test environment could also install python dependencies that are not declare in the original connector python package.

How

  • Remove the ConnectorPackageInstall step
  • Make the built connector container the input of the test step
  • Install dev/test dependencies on top of the original built image
  • Mount to the container the whole connector directory, to a /test_environment directory (the connector directory is not fully available on the built image).

The state of this "prepared" container is not kept for other steps like CAT or publish. It's just used in the python Unit / integration test step (subclasses of the PytestStep).

@alafanechere alafanechere force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from 41ce5f5 to b387477 Compare September 15, 2023 15:08
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from f9bea4b to b608d76 Compare September 15, 2023 15:08
@alafanechere alafanechere marked this pull request as ready for review September 15, 2023 15:33
@alafanechere alafanechere requested a review from a team September 15, 2023 15:33
@alafanechere alafanechere force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from b387477 to 0113f6a Compare September 17, 2023 15:01
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from b608d76 to 10448cc Compare September 17, 2023 15:16
@alafanechere alafanechere force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from 157ca30 to 894e0b9 Compare September 18, 2023 10:39
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from 10448cc to 72b1de6 Compare September 18, 2023 10:40
@alafanechere alafanechere changed the base branch from 09-14-change_our_python_connector_build_process_to_use_the_base_images to augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks September 18, 2023 10:45
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from 72b1de6 to 3fffee5 Compare September 18, 2023 10:45
@alafanechere alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch from db634ba to 8e3804a Compare September 18, 2023 14:16
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from 3fffee5 to 0a60854 Compare September 18, 2023 14:16
@alafanechere alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch from 8e3804a to 82971ce Compare September 18, 2023 14:46
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from 0a60854 to 94f6f82 Compare September 18, 2023 14:46
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from 94f6f82 to 0a1d09c Compare September 18, 2023 16:01
@alafanechere alafanechere force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from 7086a9d to 7cc99f4 Compare September 20, 2023 21:10
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch 3 times, most recently from eaf9fbc to d6d0a87 Compare September 21, 2023 11:05
@airbyte-oss-build-runner
Copy link
Collaborator

destination-duckdb test report (commit e19c6498ab) - ❌

⏲️ Total pipeline duration: 01mn27s

Step Result
Build destination-duckdb docker image for platform linux/x86_64
Unit tests
Integration tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/destination-duckdb/metadata.yaml
Connector version semver check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-duckdb test

@airbyte-oss-build-runner
Copy link
Collaborator

source-faker test report (commit e19c6498ab) - ✅

⏲️ Total pipeline duration: 01mn19s

Step Result
Build source-faker docker image for platform linux/x86_64
Unit tests
Acceptance tests
Code format checks
Validate airbyte-integrations/connectors/source-faker/metadata.yaml
Connector version semver check
QA checks

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-faker test

@alafanechere
Copy link
Contributor Author

The test results above show that we did not break the test run:

  • duckdb poetry tests are correctly executed
  • faker tests still pass

@octavia-squidington-iii octavia-squidington-iii removed the area/connectors Connector related issues label Sep 21, 2023
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from db45b8e to 28a886b Compare September 21, 2023 11:38
Copy link
Contributor

@erohmensing erohmensing left a comment

Choose a reason for hiding this comment

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

Not much to say, but I have 👀 this PR

@@ -404,6 +404,7 @@ This command runs the Python tests for a airbyte-ci poetry package.
## Changelog
| Version | PR | Description |
|---------| --------------------------------------------------------- |-----------------------------------------------------------------------------------------------------------|
| 1.6.0 | [#30474](https://github.com/airbytehq/airbyte/pull/30474) | Test connector insider their containers. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| 1.6.0 | [#30474](https://github.com/airbytehq/airbyte/pull/30474) | Test connector insider their containers. |
| 1.6.0 | [#30474](https://github.com/airbytehq/airbyte/pull/30474) | Test connectors inside their containers. |

@bnchrch bnchrch force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from 96068a5 to 18693fc Compare September 28, 2023 15:16
Base automatically changed from 09-14-change_our_python_connector_build_process_to_use_the_base_images to master October 10, 2023 17:16
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch 2 times, most recently from b14764c to 02361d2 Compare October 11, 2023 09:42
@alafanechere alafanechere force-pushed the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch from 02361d2 to 310ec11 Compare October 11, 2023 11:55
@alafanechere alafanechere changed the title [airbyte-ci] test connector inside their built container [airbyte-ci] test connectors inside their built container Oct 11, 2023
@alafanechere alafanechere enabled auto-merge (squash) October 11, 2023 12:03
@alafanechere alafanechere disabled auto-merge October 11, 2023 12:03
@alafanechere alafanechere enabled auto-merge (squash) October 11, 2023 12:06
@alafanechere alafanechere merged commit 9fae594 into master Oct 11, 2023
14 checks passed
@alafanechere alafanechere deleted the augustin/09-15-_airbyte-ci_test_connector_inside_their_built_container branch October 11, 2023 12:06
ariesgun pushed a commit to ariesgun/airbyte that referenced this pull request Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants