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

IGNITE-12302 Test ZookeeperDiscoveryTopologyChangeAndReconnectTest.testDuplicatedNodeId is broken. #6986

Closed
wants to merge 5 commits into from

Conversation

NSAmelchev
Copy link
Contributor

@NSAmelchev NSAmelchev commented Oct 17, 2019

Fixed test by disabling registering system views.
Optimized test speed by decreasing server join tries counts.
Added correct SPI exception check.

@@ -562,32 +564,23 @@ public void testForcibleClientFail() throws Exception {
* @throws Exception If failed.
*/
@Test
@WithSystemProperty(key = IGNITE_SQL_DISABLE_SYSTEM_VIEWS, value = "true")
Copy link
Contributor

Choose a reason for hiding this comment

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

IgniteH2Indexing connects to H2 instance by URL based on nodeId, so there can be more errors in indexing here in the future even if system views are disabled.
Perhaps it's better to disable indexing at all for this test (something like GridQueryProcessor.idxCls = DummyQueryIndexing.class; before each grid start).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alex-plekhanov, thanks for the suggestion. I have fixed the test. Could you take a look? TC test suite looks fine.

@Override protected void afterTest() throws Exception {
super.afterTest();

indexingDisabled = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

You should also return the value of GridQueryProcessor.idxCls to null (in case startGrid fails)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

if (indexingDisabled) {
GridQueryProcessor.idxCls = DummyQueryIndexing.class;

cfg.setSystemViewExporterSpi(new JmxSystemViewExporterSpi() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's fix SqlViewExporterSpi instead of excluding it (looks like a bug in its implementation)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alex-plekhanov, I have fixed it. Take a look, please. Tests OK.

@asfgit asfgit closed this in a6e577e Oct 22, 2019
@NSAmelchev NSAmelchev deleted the ignite-12302 branch November 26, 2019 16:52
ingvard pushed a commit to gridgain/apache-ignite that referenced this pull request Oct 14, 2020
…AndReconnectTest.testDuplicatedNodeId - Fixes apache#6986.

Signed-off-by: Aleksey Plekhanov <plehanov.alex@gmail.com>
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.

None yet

2 participants