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

explicitly add implicit dependencies of acceptance tests #1170

Merged
merged 2 commits into from
Dec 14, 2020

Conversation

cgardens
Copy link
Contributor

@cgardens cgardens commented Dec 2, 2020

What

  • AcceptanceTest.java depends on source-postgres and destination-csv. Things kinda worked before because there's always a :dev version of them somewhere, but we should make the dep explicit so that the tests always use a local dev airbyte.
  • AcceptanceTest.java depends on airbyte-commons. add the dep explicitly. previously it was getting it transitively. In practice this was annoying because intellij can't find the classes when linting every time.

@@ -25,6 +25,7 @@ configurations {

dependencies {
acceptanceTestsImplementation project(':airbyte-api')
acceptanceTestsImplementation project(':airbyte-commons')
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought airbyte-commons gets pulled in from the root level project?

@michel-tricot
Copy link
Contributor

@jrhizor how does this PR plays with your build changes?

@jrhizor jrhizor merged commit c86ea06 into master Dec 14, 2020
@jrhizor jrhizor deleted the cgardens/add_acceptance_tests_deps branch December 14, 2020 21:33
@jrhizor
Copy link
Contributor

jrhizor commented Dec 14, 2020

This didn't conflict with my changes at all.

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 this pull request may close these issues.

4 participants