IGNITE-5732 Provide API to test compatibility with old releases #2410
Conversation
daradurvs
force-pushed
the
ignite-5732-module
branch
4 times, most recently
from
8206b6c
to
5587716
Compare
daradurvs
force-pushed
the
ignite-5732-module
branch
from
6ade601
to
8190dc8
Compare
daradurvs
force-pushed
the
ignite-5732-module
branch
from
d1f47fc
to
a7003e9
Compare
daradurvs
force-pushed
the
ignite-5732-module
branch
from
a7003e9
to
9e21c89
Compare
| boolean found = ignite.cluster().node(syncId) != null; | ||
|
|
||
| if (found) | ||
| X.println(IgniteCompatibilityAbstractTest.SYNCHRONIZATION_LOG_MESSAGE_JOINED + id); |
There was a problem hiding this comment.
may be print all messages to some logger, so user can define its own way to print messages.
There was a problem hiding this comment.
No, I can't do it. I only can grab a process output of separate JVM.
It doesn't matter which logger will be used by user.
There was a problem hiding this comment.
By the same token, exchange doesn't start before cluster activation.
|
|
||
| assert GridTestUtils.waitForCondition(new GridAbsPredicate() { | ||
| @Override public boolean apply() { | ||
| return ignite.cluster().node(syncId) != null; |
There was a problem hiding this comment.
may be add awaitPartitionMapExchange()
There was a problem hiding this comment.
There is a reverse synchronization in case of starting the first node in local JVM when there are nodes in another JVM.
There was a problem hiding this comment.
By the same token, exchange doesn't start before cluster activation.
|
|
||
| exec("mvn dependency:get -Dartifact=" + artifact); | ||
|
|
||
| X.println("Download is finished"); |
| if (!defaultDBWorkDirectoryIsEmpty()) | ||
| deleteDefaultDBWorkDirectory(); | ||
|
|
||
| assert defaultDBWorkDirectoryIsEmpty() : "DB work directory is not empty."; |
There was a problem hiding this comment.
Can we assert this check?
Can asserts be disabled in test run?
| */ | ||
| public void testNodeStartByOldVersionPersistenceData() throws Exception { | ||
| try { | ||
| startGrid(1, "2.2.0", new PostConfigurationClosure(), new PostActionClosure()); |
There was a problem hiding this comment.
Can we check compatibility for implicit version?
For example - run test with nodes X-n versions?
2.2.0
2.1.0 (or 2.1.5 if it previous to 2.2.0)
2.0.0 etc.
Can we provide startPreviuousVersionGrid method for X-1 compatibility tests?
| @Override protected void beforeTest() throws Exception { | ||
| super.beforeTest(); | ||
|
|
||
| if (!defaultDBWorkDirectoryIsEmpty()) |
There was a problem hiding this comment.
Seems that we should move beforeTest and afterTest to some abstract class or utility method.
There was a problem hiding this comment.
This test class will be removed.
But, I agree we should provide PersistenceAbstractClasss, thanks!
|
|
||
| assertEquals(1, topologyVersion(ignite)); | ||
|
|
||
| startGrid("testMultiVersion", "2.3.0-SNAPSHOT", new PostConfigurationClosure()); |
There was a problem hiding this comment.
I don't like the idea to specify explicit Ignite version.
In that case we need updated test file with some script on every next version.
Can we use current version or something similar?
There was a problem hiding this comment.
This test will be removed.
There is framework-self-test with a package which installed in the Maven local repositoty.
|
|
||
| assertEquals(1, topologyVersion(ignite)); | ||
|
|
||
| startGrid(1, "2.3.0-SNAPSHOT", new PostConfigurationClosure()); |
There was a problem hiding this comment.
2.3.0-SNAPSHOT definitely should be a constant
There was a problem hiding this comment.
This is dummy test and will be removed.
| * @throws IOException In case of an error. | ||
| * @see #readClosureFromFileAndDelete(String) | ||
| */ | ||
| public static void storeToFile(@NotNull IgniteInClosure clos, @NotNull String fileName) throws IOException { |
There was a problem hiding this comment.
Why doesn't IgniteInClosure have generic parameter?
There was a problem hiding this comment.
Do you think this is required?
I think no because given instance will be serialized to a file, without any type information.
There was a problem hiding this comment.
Is storeToFile common method? Or it will be used only in compatibility module with specific closure types?
If first - we should use
public static void <T> storeToFile(@NotNull IgniteInClosure<T> clos, @NotNull String fileName) throws IOException {
If second:
public static void storeToFile(@NotNull IgniteInClosure<Ignite> clos, @NotNull String fileName) throws IOException {
There was a problem hiding this comment.
It will be used only in the compatibility module with specific closure types:
IgniteInClosure<IgniteConfiguration>
IgniteInClosure<Ignite>
| public static final String SYNCHRONIZATION_LOG_MESSAGE_PREPARED = "Remote node has prepared, id: "; | ||
|
|
||
| /** Waiting seconds of the join of a node to topology. */ | ||
| protected static final int NODE_JOIN_TIMEOUT = 30; |
There was a problem hiding this comment.
Why timeout specified in seconds?
Can we rename constant to NODE_JOIN_TIMEOUT_SEC or set a timeout in milliseconds?
There was a problem hiding this comment.
Agree. Timeout will be specified in milliseconds.
| * @see #deleteDefaultDBWorkDirectory() | ||
| */ | ||
| @SuppressWarnings("ConstantConditions") | ||
| protected boolean defaultDBWorkDirectoryIsEmpty() throws IgniteCheckedException { |
There was a problem hiding this comment.
I think method name should be isDefaultDBWorkDirectoryEmpty
| * @return Started grid. | ||
| * @throws Exception If failed. | ||
| */ | ||
| protected IgniteEx startGrid(int idx, String ver, IgniteInClosure<IgniteConfiguration> cfgClos) throws Exception { |
There was a problem hiding this comment.
Can we use some sort of enum for ver variable?
There was a problem hiding this comment.
I thought about it.
Not sure is it required or no.
| @@ -0,0 +1 @@ | |||
| org.apache.ignite.compatibility.testframework.plugins.TestCompatibilityPluginProvider | |||
There was a problem hiding this comment.
Is this provider would be available for regular test run?
If yes, we definitely should exclude it from regular test classpath.
There was a problem hiding this comment.
No, this plugin will be used only in tests in the compatibility-module and won't be used in any other module like ignite-core.
…tibilityNodeRunner
…; PostActionClosure to PostStartupClosure
…geStoreManager" This reverts commit 3f7be6a.
daradurvs
force-pushed
the
ignite-5732-module
branch
4 times, most recently
from
cdd1021
to
c683808
Compare
|
Merged to the master branch by the commit |
Special module contains testing framework which provides methods for testing backward compatibility.
It allows to start Ignite-cluster in multiversion mode. It means that nodes with different build version is allowed to join topology.