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/06 05:05:15 UTC
[GitHub] [shardingsphere] cunhazera opened a new pull request #12911: Add test 12890
cunhazera opened a new pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911
Fixes #12890.
Changes proposed in this pull request:
- Add test to ShardingSphereRuleMetaData
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937461696
@cunhazera Hi, thank you and welcome for contribution, I just add new code review comment, please check when you are free.
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937441627
@cunhazera Sure, welcome here!
I just approved to run the CI tests again, let wait for its result.
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937445531
@tristaZero I think now it's good to go.
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937462863
> @cunhazera Hi, thank you and welcome for contribution, I just add new code review comment, please check when you are free.
Done. Thank's for the review again :)
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937469880
@cunhazera Sorry, I still find a class which is not used.
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#discussion_r723863826
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/RuleConfigurationFixture.java
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.infra.metadata.rule;
+
+import org.apache.shardingsphere.infra.config.RuleConfiguration;
+
+public class RuleConfigurationFixture implements RuleConfiguration {
Review comment:
What is useful of `RuleConfigurationFixture`, It seems nobody to use 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] terrymanu commented on pull request #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-938295465
> @tristaZero @terrymanu can you review and trigger the build?
Sure, running
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-938316692
@terrymanu well, now I got the point of the static import. The CI should pass now.
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937461696
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937452337
@cunhazera Great, I just triggered CI tests. ;-)
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#discussion_r722971825
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/ShardingSphereRuleMetaDataTest.java
##########
@@ -0,0 +1,47 @@
+package org.apache.shardingsphere.infra.metadata.rule;
+
+import org.apache.shardingsphere.infra.config.RuleConfiguration;
+import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Optional;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.*;
Review comment:
Please avoid use import *
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/ClassTest1.java
##########
@@ -0,0 +1,4 @@
+package org.apache.shardingsphere.infra.metadata.rule;
+
+public final class ClassTest1 extends ShardingSphereRuleFake {
Review comment:
It is better to rename `Fake` as `Fixture`, just keep consist with original codes
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/ClassTest1.java
##########
@@ -0,0 +1,4 @@
+package org.apache.shardingsphere.infra.metadata.rule;
+
+public final class ClassTest1 extends ShardingSphereRuleFake {
Review comment:
Please avoid name class name as `ClassTest1`, it likes a temporary name.
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/ShardingSphereRuleFake.java
##########
@@ -0,0 +1,10 @@
+package org.apache.shardingsphere.infra.metadata.rule;
+
+import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
+
+public class ShardingSphereRuleFake implements ShardingSphereRule {
+ @Override
+ public String getType() {
+ return "type";
Review comment:
`type` is not make sense, could you rename it to meaningful name?
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937461306
@tristaZero yes, that's how I found the repo.
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937457573
Hi @cunhazera Dose `hacktoberfest` bring you to come here?
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#discussion_r723856644
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/MetadataRuleFixture.java
##########
@@ -0,0 +1,21 @@
+/*
+ * 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.infra.metadata.rule;
+
+public final class MetadataRuleFixture extends ShardingSphereRuleFixture {
Review comment:
What is the usage?
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-936162245
@terrymanu thanks for the review! Could you check it again?
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937441627
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-938281633
@tristaZero @terrymanu can you review and trigger the build?
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937445531
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#discussion_r723856644
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/MetadataRuleFixture.java
##########
@@ -0,0 +1,21 @@
+/*
+ * 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.infra.metadata.rule;
+
+public final class MetadataRuleFixture extends ShardingSphereRuleFixture {
Review comment:
What is the usage?
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/test/java/org/apache/shardingsphere/infra/metadata/rule/RuleConfigurationFixture.java
##########
@@ -0,0 +1,23 @@
+/*
+ * 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.infra.metadata.rule;
+
+import org.apache.shardingsphere.infra.config.RuleConfiguration;
+
+public class RuleConfigurationFixture implements RuleConfiguration {
Review comment:
What is useful of `RuleConfigurationFixture`, It seems nobody to use 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] terrymanu merged pull request #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911
--
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 #12911: Add test 12890
Posted by GitBox <gi...@apache.org>.
cunhazera commented on pull request #12911:
URL: https://github.com/apache/shardingsphere/pull/12911#issuecomment-937746221
@terrymanu yes, you're right. 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