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 2020/07/21 11:06:03 UTC
[GitHub] [shardingsphere-elasticjob] zhaoyuguang opened a new pull request #1226: Add test case for AopTargetUtils
zhaoyuguang opened a new pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226
Fixes #1212
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] dongzl commented on a change in pull request #1226: Add test case for AopTargetUtils
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226#discussion_r458504055
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public final class AopTargetUtilsTest {
+
+ @Test
+ public void assertJdkDynamicProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
+ }
+
+ @Test
+ public void assertCglibProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.setProxyTargetClass(true);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isCglibProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
Review comment:
For multiple parameter asserts, `assertThat` should be used.
Actual values of test cases should be named `actualXXX`, expected values `expectedXXX`.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public final class AopTargetUtilsTest {
+
+ @Test
+ public void assertJdkDynamicProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
Review comment:
For multiple parameter asserts, `assertThat` should be used.
Actual values of test cases should be named `actualXXX`, expected values `expectedXXX`.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/TargetJob.java
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.apache.shardingsphere.elasticjob.api.ShardingContext;
+
+public class TargetJob implements ElasticJob {
Review comment:
OK
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public final class AopTargetUtilsTest {
+
+ @Test
+ public void assertJdkDynamicProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
+ }
+
+ @Test
+ public void assertCglibProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.setProxyTargetClass(true);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isCglibProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
+ }
+
+ @Test
+ public void assertNoneProxyForGetTarget() {
+ ElasticJob proxy = new TargetJob();
+ assertFalse(AopUtils.isAopProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), proxy);
Review comment:
For multiple parameter asserts, `assertThat` should be used.
Actual values of test cases should be named `actualXXX`, expected values `expectedXXX`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] terrymanu merged pull request #1226: Add test case for AopTargetUtils
Posted by GitBox <gi...@apache.org>.
terrymanu merged pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] zhaoyuguang commented on pull request #1226: Add test case for AopTargetUtils
Posted by GitBox <gi...@apache.org>.
zhaoyuguang commented on pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226#issuecomment-662188528
Thanks for guide, I will fixed as soon
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] dongzl commented on a change in pull request #1226: Add test case for AopTargetUtils
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226#discussion_r458041987
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
Review comment:
should be `final` class.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
+
+ @Test
+ public void jdkDynamicProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
Review comment:
add four white space, keep indents consistent with the previous one.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/TargetJob.java
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.apache.shardingsphere.elasticjob.api.ShardingContext;
+
+public class TargetJob implements ElasticJob {
+
+ public void execute(final ShardingContext shardingContext) {
+
+ }
+}
Review comment:
add blank line at end of file.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
+
+ @Test
+ public void jdkDynamicProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
+ @Test
+ public void cglibProxyForGetTarget() {
Review comment:
unit test method suggest start with `assert`.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
+
+ @Test
+ public void jdkDynamicProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
+ @Test
+ public void cglibProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.setProxyTargetClass(true);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isCglibProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
+ @Test
+ public void noneProxyForGetTarget() {
+ ElasticJob proxy = new TargetJob();
+ assertFalse(AopUtils.isAopProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+}
Review comment:
add blank line at end of file.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
+
+ @Test
+ public void jdkDynamicProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
+ @Test
+ public void cglibProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.setProxyTargetClass(true);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isCglibProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
Review comment:
add four white space, keep indents consistent with the previous one.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
+
+ @Test
+ public void jdkDynamicProxyForGetTarget() {
Review comment:
unit test method suggest start with `assert`.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
+
Review comment:
add four white space, keep indents consistent with the previous one.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AopTargetUtilsTest {
+
+ @Test
+ public void jdkDynamicProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
+ @Test
+ public void cglibProxyForGetTarget() {
+ ProxyFactory pf = new ProxyFactory(new TargetJob());
+ pf.setProxyTargetClass(true);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isCglibProxy(proxy));
+ AopTargetUtils.getTarget(proxy);
+ }
+
+ @Test
+ public void noneProxyForGetTarget() {
Review comment:
unit test method suggest start with `assert`.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/TargetJob.java
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.apache.shardingsphere.elasticjob.api.ShardingContext;
+
+public class TargetJob implements ElasticJob {
Review comment:
should be `final` class.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/TargetJob.java
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.apache.shardingsphere.elasticjob.api.ShardingContext;
+
+public class TargetJob implements ElasticJob {
+
Review comment:
add four white space, keep indents consistent with the previous one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] zhaoyuguang commented on a change in pull request #1226: Add test case for AopTargetUtils
Posted by GitBox <gi...@apache.org>.
zhaoyuguang commented on a change in pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226#discussion_r458502951
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/TargetJob.java
##########
@@ -0,0 +1,28 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.apache.shardingsphere.elasticjob.api.ShardingContext;
+
+public class TargetJob implements ElasticJob {
Review comment:
CGLIB testcase use it，so this not add
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] dongzl commented on a change in pull request #1226: Add test case for AopTargetUtils
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226#discussion_r458504029
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public final class AopTargetUtilsTest {
+
+ @Test
+ public void assertJdkDynamicProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
Review comment:
For multiple parameter asserts, `assertThat` should be 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere-elasticjob] dongzl commented on a change in pull request #1226: Add test case for AopTargetUtils
Posted by GitBox <gi...@apache.org>.
dongzl commented on a change in pull request #1226:
URL: https://github.com/apache/shardingsphere-elasticjob/pull/1226#discussion_r458504055
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public final class AopTargetUtilsTest {
+
+ @Test
+ public void assertJdkDynamicProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
+ }
+
+ @Test
+ public void assertCglibProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.setProxyTargetClass(true);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isCglibProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
Review comment:
For multiple parameter asserts, `assertThat` should be used.
##########
File path: elasticjob-lite/elasticjob-lite-spring/elasticjob-lite-spring-namespace/src/test/java/org/apache/shardingsphere/elasticjob/lite/spring/namespace/job/util/AopTargetUtilsTest.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.elasticjob.lite.spring.namespace.job.util;
+
+import org.apache.shardingsphere.elasticjob.api.ElasticJob;
+import org.junit.Test;
+import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.aop.support.AopUtils;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public final class AopTargetUtilsTest {
+
+ @Test
+ public void assertJdkDynamicProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.addInterface(ElasticJob.class);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isJdkDynamicProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
+ }
+
+ @Test
+ public void assertCglibProxyForGetTarget() {
+ ElasticJob target = new TargetJob();
+ ProxyFactory pf = new ProxyFactory(target);
+ pf.setProxyTargetClass(true);
+ ElasticJob proxy = (ElasticJob) pf.getProxy();
+ assertTrue(AopUtils.isCglibProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), target);
+ }
+
+ @Test
+ public void assertNoneProxyForGetTarget() {
+ ElasticJob proxy = new TargetJob();
+ assertFalse(AopUtils.isAopProxy(proxy));
+ assertEquals(AopTargetUtils.getTarget(proxy), proxy);
Review comment:
For multiple parameter asserts, `assertThat` should be 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org