You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/10/05 17:00:38 UTC
[GitHub] [shardingsphere] cunhazera opened a new pull request #12909: Adding test to ConnectionTransaction
cunhazera opened a new pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909
Fixes #12889.
Changes proposed in this pull request:
- Adding test to `ConnectionTransaction`
If I can add more TransactionContexts would be possible to have more test cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction.getDistributedTransactionOperationType()
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-938589791
@terrymanu thanks for the approval! Is this CI error something random?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-938017729
> The test cases are not finished for the class ConnectionTransaction. So the this PR can not close the issue #12889. For more clearly, please the title of this pull request.
What exactly do you mean? Should I add more test cases?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#discussion_r722854370
##########
File path: shardingsphere-kernel/shardingsphere-transaction/shardingsphere-transaction-core/src/test/java/org/apache/shardingsphere/transaction/ConnectionTransactionTest.java
##########
@@ -0,0 +1,38 @@
+package org.apache.shardingsphere.transaction;
+
+import org.apache.shardingsphere.infra.database.DefaultSchema;
+import org.apache.shardingsphere.transaction.ConnectionTransaction.DistributedTransactionOperationType;
+import org.apache.shardingsphere.transaction.config.TransactionRuleConfiguration;
+import org.apache.shardingsphere.transaction.context.TransactionContexts;
+import org.apache.shardingsphere.transaction.rule.TransactionRule;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Map;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public final class ConnectionTransactionTest {
+
+ private ConnectionTransaction connectionTransaction;
+
+ @Before
+ public void init() {
+ Map<String, ShardingSphereTransactionManagerEngine> actualEngines = Collections.singletonMap(DefaultSchema.LOGIC_NAME, new ShardingSphereTransactionManagerEngine());
+ TransactionContexts transactionContexts = new TransactionContexts(actualEngines);
+ connectionTransaction = new ConnectionTransaction(
+ DefaultSchema.LOGIC_NAME,
+ new TransactionRule(new TransactionRuleConfiguration("XA", "Atomikos")),
+ transactionContexts
+ );
+ }
+
+ @Test
+ public void testDistributedTransactionOperationTypeIgnore() {
Review comment:
Name it `assertXXX`.
##########
File path: shardingsphere-kernel/shardingsphere-transaction/shardingsphere-transaction-core/src/test/java/org/apache/shardingsphere/transaction/ConnectionTransactionTest.java
##########
@@ -0,0 +1,38 @@
+package org.apache.shardingsphere.transaction;
+
+import org.apache.shardingsphere.infra.database.DefaultSchema;
+import org.apache.shardingsphere.transaction.ConnectionTransaction.DistributedTransactionOperationType;
+import org.apache.shardingsphere.transaction.config.TransactionRuleConfiguration;
+import org.apache.shardingsphere.transaction.context.TransactionContexts;
+import org.apache.shardingsphere.transaction.rule.TransactionRule;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Map;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public final class ConnectionTransactionTest {
+
+ private ConnectionTransaction connectionTransaction;
+
+ @Before
+ public void init() {
+ Map<String, ShardingSphereTransactionManagerEngine> actualEngines = Collections.singletonMap(DefaultSchema.LOGIC_NAME, new ShardingSphereTransactionManagerEngine());
+ TransactionContexts transactionContexts = new TransactionContexts(actualEngines);
+ connectionTransaction = new ConnectionTransaction(
+ DefaultSchema.LOGIC_NAME,
+ new TransactionRule(new TransactionRuleConfiguration("XA", "Atomikos")),
+ transactionContexts
+ );
+ }
+
+ @Test
+ public void testDistributedTransactionOperationTypeIgnore() {
+ DistributedTransactionOperationType operationType = connectionTransaction.getDistributedTransactionOperationType(false);
+ assertThat(operationType, equalTo(DistributedTransactionOperationType.IGNORE));
+ }
+
Review comment:
Remove the redundant blank line.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-936151024
@tristaZero I added license to the test file, but there are some errors with the checkstyle.
It's saying to avoid static import when using `MatcherAssert.assertThat`, but most `assertThat` are imported statically.
Can you check it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera removed a comment on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera removed a comment on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937442108
@tristaZero It says: AvoidStaticImport: Using a static member import should be avoided - org.hamcrest.MatcherAssert.assertThat.
But most `MatcherAssert` are statically imported. Any thoughts on that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937439239
Sorry, @cunhazera I approved to run CI tests, but it seems like some issues still exist. Could you have a look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937439239
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937442589
@tristaZero I'll try to remove the static import.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-938332289
> > The test cases are not finished for the class ConnectionTransaction. So the this PR can not close the issue #12889. For more clearly, please the title of this pull request.
>
> What exactly do you mean? Should I add more test cases?
Or just change the title to "Adding test to ConnectionTransaction.getDistributedTransactionOperationType()"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937468482
The test cases are not finished for the class ConnectionTransaction.
So the this PR can not close the issue #12889.
For more clearly, please the title of this pull request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero merged pull request #12909: Adding test to ConnectionTransaction.getDistributedTransactionOperationType()
Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937451572
Sure, I just made CI run again, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937442108
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937468482
The test cases are not finished for the class ConnectionTransaction.
So the this PR can not close the issue #12889.
For more clearly, please the title of this pull request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937444051
@tristaZero can you run it again? I'm ready to add more tests, I just want to know If I'm on the right way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on a change in pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#discussion_r723861917
##########
File path: shardingsphere-kernel/shardingsphere-transaction/shardingsphere-transaction-core/src/test/java/org/apache/shardingsphere/transaction/ConnectionTransactionTest.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.transaction;
+
+import org.apache.shardingsphere.infra.database.DefaultSchema;
+import org.apache.shardingsphere.transaction.ConnectionTransaction.DistributedTransactionOperationType;
+import org.apache.shardingsphere.transaction.config.TransactionRuleConfiguration;
+import org.apache.shardingsphere.transaction.context.TransactionContexts;
+import org.apache.shardingsphere.transaction.rule.TransactionRule;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Map;
+
+public final class ConnectionTransactionTest {
+
+ private ConnectionTransaction connectionTransaction;
+
+ @Before
+ public void init() {
+ Map<String, ShardingSphereTransactionManagerEngine> actualEngines = Collections.singletonMap(DefaultSchema.LOGIC_NAME, new ShardingSphereTransactionManagerEngine());
+ TransactionContexts transactionContexts = new TransactionContexts(actualEngines);
+ connectionTransaction = new ConnectionTransaction(
+ DefaultSchema.LOGIC_NAME,
+ new TransactionRule(new TransactionRuleConfiguration("XA", "Atomikos")),
+ transactionContexts
+ );
+ }
+
+ @Test
+ public void assertDistributedTransactionOperationTypeIgnore() {
+ DistributedTransactionOperationType operationType = connectionTransaction.getDistributedTransactionOperationType(false);
+ MatcherAssert.assertThat(operationType, Matchers.equalTo(DistributedTransactionOperationType.IGNORE));
Review comment:
Please use static import with `assertThat`
FYI: https://shardingsphere.apache.org/community/en/contribute/code-conduct/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937442108
@tristaZero It says: AvoidStaticImport: Using a static member import should be avoided - org.hamcrest.MatcherAssert.assertThat.
But most `MatcherAssert` are statically imported. Any thoughts on that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction.getDistributedTransactionOperationType()
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-938335880
> > > The test cases are not finished for the class ConnectionTransaction. So the this PR can not close the issue #12889. For more clearly, please the title of this pull request.
> >
> >
> > What exactly do you mean? Should I add more test cases?
>
> Or just change the title to "Adding test to ConnectionTransaction.getDistributedTransactionOperationType()"?
Alright, it's done. And the checkstyle is fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera commented on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-935412888
@TeslaCN can you review pls?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] cunhazera removed a comment on pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
cunhazera removed a comment on pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#issuecomment-937442108
@tristaZero It says: AvoidStaticImport: Using a static member import should be avoided - org.hamcrest.MatcherAssert.assertThat.
But most `MatcherAssert` are statically imported. Any thoughts on that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] terrymanu commented on a change in pull request #12909: Adding test to ConnectionTransaction
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #12909:
URL: https://github.com/apache/shardingsphere/pull/12909#discussion_r723861917
##########
File path: shardingsphere-kernel/shardingsphere-transaction/shardingsphere-transaction-core/src/test/java/org/apache/shardingsphere/transaction/ConnectionTransactionTest.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.transaction;
+
+import org.apache.shardingsphere.infra.database.DefaultSchema;
+import org.apache.shardingsphere.transaction.ConnectionTransaction.DistributedTransactionOperationType;
+import org.apache.shardingsphere.transaction.config.TransactionRuleConfiguration;
+import org.apache.shardingsphere.transaction.context.TransactionContexts;
+import org.apache.shardingsphere.transaction.rule.TransactionRule;
+import org.hamcrest.MatcherAssert;
+import org.hamcrest.Matchers;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collections;
+import java.util.Map;
+
+public final class ConnectionTransactionTest {
+
+ private ConnectionTransaction connectionTransaction;
+
+ @Before
+ public void init() {
+ Map<String, ShardingSphereTransactionManagerEngine> actualEngines = Collections.singletonMap(DefaultSchema.LOGIC_NAME, new ShardingSphereTransactionManagerEngine());
+ TransactionContexts transactionContexts = new TransactionContexts(actualEngines);
+ connectionTransaction = new ConnectionTransaction(
+ DefaultSchema.LOGIC_NAME,
+ new TransactionRule(new TransactionRuleConfiguration("XA", "Atomikos")),
+ transactionContexts
+ );
+ }
+
+ @Test
+ public void assertDistributedTransactionOperationTypeIgnore() {
+ DistributedTransactionOperationType operationType = connectionTransaction.getDistributedTransactionOperationType(false);
+ MatcherAssert.assertThat(operationType, Matchers.equalTo(DistributedTransactionOperationType.IGNORE));
Review comment:
Please use static import with `assertThat`
FYI: https://shardingsphere.apache.org/community/en/contribute/code-conduct/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org