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.

Sign up for GitHub

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

Jump to bottom

Ref#4772 Introduce group testing for different jdbc db types #5116

Merged
merged 1 commit into from
Aug 10, 2023
Merged

Ref#4772 Introduce group testing for different jdbc db types #5116

merged 1 commit into from
Aug 10, 2023

Conversation

ldrozdo
Copy link
Contributor

@ldrozdo ldrozdo commented Jul 24, 2023

Fix #4772

zhfeng
zhfeng reviewed Jul 24, 2023
View reviewed changes
integration-test-groups/jdbc/db2/src/main/resources/reflection-config.json Outdated Show resolved Hide resolved
integration-test-groups/jdbc/db2/src/main/resources/application.properties Outdated

quarkus.datasource.mssql.db-kind=mssql
quarkus.datasource.mssql.jdbc.max-size=8

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we only keep quarkus.datasource.db2 configurations sine it is for db2 testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhfeng I thought that the application.properties file should be the same for all modules, otherwise group tests don't work properly.

@zhfeng
Copy link
Contributor

zhfeng commented Jul 24, 2023

Thanks @ldrozdo for working on this issue. I just add some comments.

jamesnetherton
jamesnetherton reviewed Jul 26, 2023
View reviewed changes
Copy link
Contributor

@jamesnetherton jamesnetherton left a comment

Choose a reason for hiding this comment

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

Should we not be removing the integration-tests/jdbc module? And also its reference in test-categories.yaml.

@ldrozdo
Copy link
Contributor Author

ldrozdo commented Jul 26, 2023

@jamesnetherton honestly, I'm not sure what is the right process. But our QE job for database certification already uses integration-tests/jdbc, so I would leave it like this at least for a while.

@ldrozdo ldrozdo force-pushed the jdbc-grouped-tests branch 3 times, most recently from 85e6a98 to e1a1028 Compare August 3, 2023 06:50
@ldrozdo
Copy link
Contributor Author

ldrozdo commented Aug 3, 2023

Hi @zhfeng , can you please help me? I managed to fix the failing checks, I think failings in native tests are glitches because before, they were already succesfull... But alternative-platform tests are still failing: Could not find a valid Docker environment. Please check configuration. Attempted configurations were: NpipeSocketClientProviderStrategy: failed with exception InvalidConfigurationException (windows containers are currently not supported)As no valid configuration was found, execution cannot continue. Is there something I can do to fix it?

@jamesnetherton
Copy link
Contributor

Is there something I can do to fix it?

There's no Docker on the Windows build. You can add a profile like this one so the test gets skipped on that platform:

https://github.com/apache/camel-quarkus/blob/main/integration-test-groups/azure/azure-storage-blob/pom.xml#L165-L175

ppalaga
ppalaga reviewed Aug 4, 2023
View reviewed changes
integration-test-groups/jdbc/h2/pom.xml Outdated
Comment on lines 61 to 88
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jdbc-db2</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jdbc-h2</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jdbc-mariadb</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jdbc-mssql</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jdbc-mysql</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jdbc-oracle</artifactId>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-jdbc-postgresql</artifactId>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all these JDBC artifacts needed in this specific test module? We generally prefer keeping the minimal set of dependencies. The same issue seems to exist in other submodules of integration-test-groups/jdbc

@ppalaga
Copy link
Contributor

ppalaga commented Aug 4, 2023

@jamesnetherton honestly, I'm not sure what is the right process. But our QE job for database certification already uses integration-tests/jdbc, so I would leave it like this at least for a while.

I guess, after merging this change, you want to run tests in all submodules of integration-test-groups/jdbc.

@ldrozdo
Copy link
Contributor Author

ldrozdo commented Aug 9, 2023

@jamesnetherton thank you, the profile helped.
@ppalaga thanks, I've fixed the dependencies.
About the deleting integration-tests/jdbc, I will create a task for it and I will make sure the deletion won't affect our jobs.

I have one (hopefully last) problem. The native tests were already passing, but now they've started to fail with:
Caused by: com.github.dockerjava.api.exception.DockerClientException: Could not pull image: failed to register layer: ApplyLayer exit status 1 stdout: stderr: write /usr/lib/x86_64-linux-gnu/libLLVM-14.so.1: no space left on device
Please, is there something I can do about it?

@jamesnetherton
Copy link
Contributor

jamesnetherton commented Aug 9, 2023 •
edited
Loading

is there something I can do about it

I guess we're eating up a lot of disk space with all of the container images we pull....

I can create a separate PR to try and reclaim some space before the tests run.

#5159

@jamesnetherton
Copy link
Contributor

@ldrozdo if you rebase with the latest work from main, the disk space issue should hopefully be resolved.

@jamesnetherton jamesnetherton merged commit a3016ad into apache:main Aug 10, 2023
20 checks passed
@jamesnetherton
Copy link
Contributor

Follow up issue to remove the old itest module #5164.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jamesnetherton jamesnetherton jamesnetherton left review comments

@ppalaga ppalaga ppalaga left review comments

@zhfeng zhfeng zhfeng left review comments

@oscerd oscerd oscerd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Introduce group testing for different jdbc db types
5 participants
@ldrozdo @zhfeng @jamesnetherton @ppalaga @oscerd