Skip to content

Add Tarantool 3 support and matrix tests #114

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

Merged
merged 6 commits into from
Feb 1, 2024
Merged

Conversation

iDneprov
Copy link
Contributor

@iDneprov iDneprov commented Jan 30, 2024

  • Add tarantool 3 support
  • Add matrix tests with tarantool 1.x, 2.11.2 and 3.0.1
  • Change TARANTOOL_VERSION usage (remove -centos7 from template for Tarantool 3 images)
  • Add tarantool-container test profile for matrix tests
  • Update DEFAULT_IMAGE_VERSION to 2.11.2

I haven't forgotten about:

  • Tests
  • Changelog
  • Documentation
  • Commit messages comply with the guideline
  • Cleanup the code for review. See checklist

Related issues:

Closes #109

Copy link

Overall Project 74.82% 🍏
Files changed 100% 🍏

File Coverage
TarantoolContainerClientHelper.java 80.47% 🍏
TarantoolContainer.java 68.87% 🍏

Add tarantool 3 support
Add matrix tests with tarantool 1.x, 2.11.2 and 3.0.1
Change TARANTOOL_VERSION usage (remove -centos7 from template)
Add tarantool-container profile for matrix tests
Update DEFAULT_IMAGE_VERSION to 2.11.2

Closes #109
@iDneprov iDneprov requested review from ArtDu and nickkkccc January 31, 2024 21:05
@iDneprov iDneprov changed the title wip Add Tarantool 3 support and matrix tests Jan 31, 2024
Copy link

Overall Project 74.82% 🍏
Files changed 100% 🍏

File Coverage
TarantoolContainerClientHelper.java 80.47% 🍏
TarantoolContainer.java 68.87% 🍏

@@ -73,31 +74,14 @@ public void testContainerWithParameters() throws Exception {

@Test
public void testContainerWithTrueVersion() throws Exception {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In matrix tests env var TARANTOOL_VERSION always set. So this test is redundant.

dkasimovskiy
dkasimovskiy previously approved these changes Feb 1, 2024
List<String> result;
try (TarantoolContainer container = new TarantoolContainer()) {
container.start();
result = container.executeCommandDecoded("return _TARANTOOL");
}

assertEquals(1, result.size());
assertTrue(result.get(0).startsWith(TarantoolContainer.DEFAULT_IMAGE_VERSION));
assertTrue(result.get(0).startsWith(String.valueOf(tarantoolVersion.charAt(0))));
Copy link
Contributor

Choose a reason for hiding this comment

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

What if these are not CI tests with predefined Tarantool versions, and there is no TARANTOOL_VERSION variable on the system on which the test runs? It seems that a NullPointerException is possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added if statement

Copy link

github-actions bot commented Feb 1, 2024

Overall Project 74.82% 🍏
Files changed 100% 🍏

File Coverage
TarantoolContainerClientHelper.java 80.47% 🍏
TarantoolContainer.java 68.87% 🍏

dkasimovskiy
dkasimovskiy previously approved these changes Feb 1, 2024
@@ -32,6 +32,7 @@ jobs:
env:
TARANTOOL_SERVER_USER: root
TARANTOOL_SERVER_GROUP: root
TARANTOOL_VERSION: "2.11.2-centos7"
Copy link
Contributor

Choose a reason for hiding this comment

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

why didn't you delete this job? It's duplicate as a part of matrix tests

Copy link
Contributor Author

@iDneprov iDneprov Feb 1, 2024

Choose a reason for hiding this comment

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

I'v removed TrantoolCintainer classes from integration profile

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Then rename jobs, it's confusing because matrix tests are also integration tests

Copy link
Contributor

Choose a reason for hiding this comment

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

tests-tarantool-container

tests-cartridge-container

tests-ee

Comment on lines +49 to +50
"\" > container-tmp.lua &&" +
" tarantool container-tmp.lua";
Copy link
Contributor

Choose a reason for hiding this comment

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

describe this in comments if you didn't

Copy link

github-actions bot commented Feb 1, 2024

Overall Project 74.82% 🍏
Files changed 100% 🍏

File Coverage
TarantoolContainerClientHelper.java 80.47% 🍏
TarantoolContainer.java 68.87% 🍏

@dkasimovskiy dkasimovskiy merged commit 493e291 into master Feb 1, 2024
@dkasimovskiy dkasimovskiy deleted the gh-109-tarantool-3 branch February 1, 2024 16:05
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.

Add tarantool 3.0 support
4 participants