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

ksql-functional-test module depends on test-jars #2804

Closed
big-andy-coates opened this issue May 10, 2019 · 8 comments · Fixed by #3421
Closed

ksql-functional-test module depends on test-jars #2804

big-andy-coates opened this issue May 10, 2019 · 8 comments · Fixed by #3421
Assignees
Milestone

Comments

@big-andy-coates
Copy link
Contributor

The new ksql-functional-tests module, which I understand will be what customers will use to test KSQL statements, currently depends on test-jars from other modules, e.g.

<dependency>
      <groupId>io.confluent.ksql</groupId>
      <artifactId>ksql-engine</artifactId>
      <version>${project.version}</version>
      <type>test-jar</type>
</dependency>

We don't really want to be shipping jars to customers that rely on test jars, so we should break all these dependencies.

@big-andy-coates big-andy-coates added this to the 5.3 milestone May 10, 2019
@apurvam
Copy link
Contributor

apurvam commented Jun 7, 2019

Ping @hjafarpour do we plan to take care of this?

@hjafarpour
Copy link
Contributor

Removing those dependencies will require code duplication since we would need to have the classes from the tests in the new ksql-functional-test module.
In this case, I think it's better to have code duplication instead of depending on the test-jars. Since we passed code freeze for 5.3, I'll keep it as it in 5.3 and make the refactoring for 5.4.

@apurvam
Copy link
Contributor

apurvam commented Jul 26, 2019

I didn't dig into the details, but there probably are more options than full code duplication or depending on test jars.

Either way, given the other problems this has caused (like test UDFS showing up in the list), it is probably better to figure out a fix and do it on 5.3. The next bug fix is in a few weeks. we should aim to get this done by then.

@big-andy-coates
Copy link
Contributor Author

One of the tickets associated with #3252

@big-andy-coates
Copy link
Contributor Author

@purplefox, we were hoping you may be able to help with this...

@purplefox
Copy link
Contributor

Another possibility would be to separate out the shared test util classes into a new module, and have ksql-functional-tests and any tests from other modules that require them to depend on that.

@purplefox
Copy link
Contributor

Or move them to ksql-test-utils

@vpapavas
Copy link
Member

I think separating them to a different module is a good idea. But I would like someone with more engineering experience to chip in as well.

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 a pull request may close this issue.

5 participants