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

HerdDB profile for integration tests #79

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

eolivelli
Copy link
Contributor

@eolivelli eolivelli commented Apr 10, 2021

This is a draft, I am going to add a profile that adds HerdDB 0.20.0 dependency and start HerdDB server in embeded in-memory only mode.

https://github.com/diennea/herddb/wiki/Installing-and-running-HerdDB

https://github.com/diennea/herddb/wiki/Using-JDBC-Driver

@eolivelli
Copy link
Contributor Author

@struberg @rmannibucau FYI
I am just starting...
I hope we can have something useful to test the release

</dependency>
</dependencies>
<properties>
<connection.driver.name>herddb.jdbc.Driver</connection.driver.name>
Copy link
Contributor

Choose a reason for hiding this comment

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

what about a plain java test since herddb is java? docker is mainly an additional test to enable to test which looks not needed here, wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

For me docker is also fine. The benefit of a docker image is that you drop it and have a very well defined initial status.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the main problem is about adding more jars in the classpath.

I am fine with using in-memory db as well, tests will be probably faster?

we should have both.

I would start with docker in order to not need to spend time in debugging potential problems about re-initialization of the server

Copy link
Contributor

Choose a reason for hiding this comment

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

I would dependency:unpack the zip/stack and just launch it with a dedicated classloader (no conflict wth openjpa)
Docker is fine but don't expect it to be ran often except on the CI from time to time - this is my main point, only Mark runs docker profiles AFAIK.

@eolivelli
Copy link
Contributor Author

@rmannibucau @struberg I am able to run the tests using HerdDB is pure "embedded" mode.
the problem is that there are many tests that are failing.

When I developed the fixes for OpenJPA I did not run all of the tests, but only a basic subset.

I have updated the patch, with -Dtest-herddb you run the tests using HerdDB
mvn clean install -Dtest-herddb

@eolivelli
Copy link
Contributor Author

I added a few references to the docs about the ways to run HerdDB

probably many failures are due to:

  • missing using of "delimiters"
  • unsupported syntaxes in HerdDB (all in all it is a new dialect....)

This is a minimal test case project we have in HerdDB repository to ensure that OpenJPA is working with the basic features
https://github.com/diennea/herddb/tree/master/herddb-thirdparty/openjpa-test

@rmannibucau
Copy link
Contributor

We can use assume to skip unsupported tests I guess for now and flag them explicitly as "todo" maybe? (For next release to not block it)

@eolivelli
Copy link
Contributor Author

@rmannibucau @struberg I am adding assumptions in order to skip test that fail to HerdDB.

some tests fail due to assertions that do not keep into consideration that the dictionary is applying delimiters, some tests are due to some other syntax error (probably something not supported by Calcite) and some other errors fail due to some DB error that needs more investigation.

public class TestUnwrap extends SingleEMFTestCase {

@Override
protected void setUp(Object... props) {
assumeFalse(this.getDBDictionary() instanceof HerdDBDictionary);
Copy link
Member

Choose a reason for hiding this comment

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

do you just want to skip this test for HerdDB? In which case you can simply call setUnsupportedDatabases(HerdDBDictionary.class)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@struberg thanks for the hint.
I wasn't aware of setUnsupportedDatabases

I will rework this patch

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.

3 participants