airbyte icon indicating copy to clipboard operation
airbyte copied to clipboard

Better document and unify SAT tests runs

Open grubberr opened this issue 2 years ago • 1 comments

There a at least 3 places which describe how to execute SAT tests:

  1. Connectors documentation: https://github.com/airbytehq/airbyte/blob/master/airbyte-integrations/connectors/source-github/README.md
python -m pytest integration_tests -p integration_tests.acceptance
  1. gradle task "sourceAcceptanceTest" https://github.com/airbytehq/airbyte/blob/master/buildSrc/src/main/groovy/airbyte-source-acceptance-test.gradle#L22

  2. acceptance-test-docker.sh script https://github.com/airbytehq/airbyte/blob/master/airbyte-integrations/connectors/source-github/acceptance-test-docker.sh#L15

./acceptance-test-docker.sh

The reason why I created this issue is that 3-rd option differs from 1-st and 2-nd options. Option [1] and [2] run SAT tests via Integration tests using this file integration_tests.acceptance.py but option [3] runs SAT tests without using integration_tests.acceptance.py. It can confuse people, or if we set up some additional initialization steps in integration_tests.acceptance.py these steps will not work in 3-rd option.

We need to unify all 3 options.

grubberr avatar Nov 08 '22 19:11 grubberr

Clearly document which to use depending on what you need to do, since there are different behaviors:

  • [1] Is useful when developing SAT since it runs the python directly without going through docker
  • [2] is ran on CI and uses docker
  • [3] is funcionally equivalent to 2?

We should update the docs to indicate which one should be used depending on the intent.

Recommendation is to:

  • use the gradle task if you are developing a connector
  • use the python one if you are developing SAT
  • .sh is useful for developing outside of the monorepo, but thats not really a case we support right now. Also helpful to quickly swithc the version. Decide on whether to keep or deprecate. An option could be to remove the scripts for each connector and make a better one that takes some args and is global in tools

pedroslopez avatar Nov 29 '22 18:11 pedroslopez

Closing this issues because the new happy path will be our new connectors CLI for running tests within dagger.

cc @alafanechere - is there a story to clean up all these old utils from connectors once we get dagger up and running?

evantahler avatar Mar 30 '23 22:03 evantahler