[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Flink Jira Bot (Jira)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17336763#comment-17336763
 ] 

Flink Jira Bot commented on FLINK-6444:
---

This issue was labeled "stale-major" 7 ago and has not received any updates so 
it is being deprioritized. If this ticket is actually Major, please raise the 
priority and ask a committer to assign you the issue or revive the public 
discussion.


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Priority: Major
>  Labels: pull-request-available, stale-major, starter
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Flink Jira Bot (Jira)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17328786#comment-17328786
 ] 

Flink Jira Bot commented on FLINK-6444:
---

This major issue is unassigned and itself and all of its Sub-Tasks have not 
been updated for 30 days. So, it has been labeled "stale-major". If this ticket 
is indeed "major", please either assign yourself or give an update. Afterwards, 
please remove the label. In 7 days the issue will be deprioritized.

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Priority: Major
>  Labels: pull-request-available, stale-major, starter
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16664278#comment-16664278
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

zentol closed pull request #4705: [FLINK-6444] [build] Add a check that 
'@VisibleForTesting' methods ar…
URL: https://github.com/apache/flink/pull/4705
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git 
a/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
 
b/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
index 46c821edfa2..eb7dc3106f6 100644
--- 
a/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
+++ 
b/flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
@@ -439,7 +439,7 @@ public CheckpointingMode getCheckpointingMode() {
 * from operations on {@link 
org.apache.flink.streaming.api.datastream.KeyedStream}) is maintained
 * (heap, managed memory, externally), and where state 
snapshots/checkpoints are stored, both for
 * the key/value state, and for checkpointed functions (implementing 
the interface
-* {@link org.apache.flink.streaming.api.checkpoint.Checkpointed}).
+* {@link org.apache.flink.streaming.api.checkpoint.ListCheckpointed}).
 *
 * The {@link 
org.apache.flink.runtime.state.memory.MemoryStateBackend} for example
 * maintains the state in heap memory, as objects. It is lightweight 
without extra dependencies,
diff --git 
a/flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 
b/flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
new file mode 100644
index 000..cb5f2cbecba
--- /dev/null
+++ 
b/flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
@@ -0,0 +1,68 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   ConfigurationBuilder configurationBuilder = new 
ConfigurationBuilder()
+   
.useParallelExecutor(Runtime.getRuntime().availableProcessors())
+   
.addUrls(ClasspathHelper.forPackage("org.apache.flink.core"))
+   .addScanners(
+   new MemberUsageScanner(),
+   new MethodAnnotationsScanner());
+
+   final Reflections reflections = new 
Reflections(configurationBuilder);
+
+   Set methods = 
reflections.getMethodsAnnotatedWith(VisibleForTesting.class);
+
+   for (Method method : methods) {
+   Set usages = reflections.getMethodUsage(method);
+   for (Member member : usages) {
+   if (member instanceof Method) {
+   Method visibleForTestingUsageScope = 
(Method) member;
+   if 

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Piotr Nowojski (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16210982#comment-16210982
 ] 

Piotr Nowojski commented on FLINK-6444:
---

Hi, sorry for not looking here earlier. As I have written in the pull request:

Hmmm, I looked a little bit and it seems like there are tools doing this:
https://github.com/google/guava/issues/1640
https://github.com/WorksApplications/findbugs-plugin
https://github.com/WorksApplications/findbugs-plugin/commit/d77ca84a9dcc8a99a99a155423c5e7b707a1a5fc

It evens seems to be working based on 
{{
 * A detector to ensure that implementation (class in src/main/java) doesn't 
call
 * package-private method in other class which is annotated by {@code 
@VisibleForTesting}.
}}

If above doesn't work/fit us, we could probably easily implement our own check 
for that using 
[findbugs-maven-plugin](https://gleclaire.github.io/findbugs-maven-plugin/usage.html),
 maybe like described 
[here](https://writeoncereadmany.wordpress.com/2016/04/08/how-to-find-bugs-part-3-visiblefortesting/).

Could you try out WorksApplications/findbugs-plugin? The issue might be that 
they are using guava's {{VisibleForTesting}}, but maybe it is configurable? 

Does it work, if we create new flink module with an implementation of some 
maven plugin and then use this plugin in our build?

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Hai Zhou UTC+8 (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16210831#comment-16210831
 ] 

Hai Zhou UTC+8 commented on FLINK-6444:
---

Hi [~pnowojski], You could take a look at the above discussion. I wrote a 
`flink-spotbugs-plugin` maven plugin with a detector to check this.  although 
it is very simple to use, but I do not know where to put it and how to deploy 
to the public maven repository.   
I'd like to hear your thoughts.

code commit: 
https://github.com/yew1eb/flink/commit/d8edc6625ad98302cca54d80c5321d7fe3948b82

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16210810#comment-16210810
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user pnowojski commented on the issue:

https://github.com/apache/flink/pull/4705
  
Hmmm, I looked a little bit and it seems like there are tools doing this:
https://github.com/google/guava/issues/1640
https://github.com/WorksApplications/findbugs-plugin

https://github.com/WorksApplications/findbugs-plugin/commit/d77ca84a9dcc8a99a99a155423c5e7b707a1a5fc

It evens seems to be working based on 
```
 * A detector to ensure that implementation (class in src/main/java) 
doesn't call
 * package-private method in other class which is annotated by {@code 
@VisibleForTesting}.
```

If above doesn't work/fit us, we could probably easily implement our own 
check for that using 
[findbugs-maven-plugin](https://gleclaire.github.io/findbugs-maven-plugin/usage.html),
 maybe like described 
[here](https://writeoncereadmany.wordpress.com/2016/04/08/how-to-find-bugs-part-3-visiblefortesting/).

Nevertheless I think this feature should be first discussed on a dev 
mailing list. I am not against it (as long as it will not significantly prolong 
builds), but it would be better to discuss this with the community. 


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16210479#comment-16210479
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on the issue:

https://github.com/apache/flink/pull/4705
  
@pnowojski I tried. But It seems that I can not implement that 
functionality. I think it is very strange to find it's source code comes from. 
@StephanEwen  Could you give me some idea on this ? Thanks.


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16209563#comment-16209563
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on the issue:

https://github.com/apache/flink/pull/4705
  
I have got an idea for how to get it's source code whether comes from 
src/test. Since I can get it's class name. Then I use this name to search in 
flink project. and judge where it comes from and whether this path contains 
src/test and something like this. I think it works. @pnowojski What do you 
think ?


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16209505#comment-16209505
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145445707
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
+   
.useParallelExecutor(Runtime.getRuntime().availableProcessors())
+   .addUrls(ClasspathHelper.forPackage("org.apache.flink"))
+   .addScanners(new MemberUsageScanner(),
+   new MethodAnnotationsScanner()));
+
+   Set methods = 
reflections.getMethodsAnnotatedWith(VisibleForTesting.class);
+
+   for (Method method : methods) {
+   Set usages = reflections.getMethodUsage(method);
+   for (Member member : usages) {
+   if (member instanceof Method) {
+   Method methodHopeWithTestAnnotation = 
(Method) member;
+   if 
(!methodHopeWithTestAnnotation.isAnnotationPresent(Test.class)) {
--- End diff --

@pnowojski I think I need to think again for how to get src/test path.


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16209489#comment-16209489
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145443119
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
+   
.useParallelExecutor(Runtime.getRuntime().availableProcessors())
+   .addUrls(ClasspathHelper.forPackage("org.apache.flink"))
+   .addScanners(new MemberUsageScanner(),
+   new MethodAnnotationsScanner()));
+
+   Set methods = 
reflections.getMethodsAnnotatedWith(VisibleForTesting.class);
+
+   for (Method method : methods) {
+   Set usages = reflections.getMethodUsage(method);
+   for (Member member : usages) {
+   if (member instanceof Method) {
+   Method methodHopeWithTestAnnotation = 
(Method) member;
+   if 
(!methodHopeWithTestAnnotation.isAnnotationPresent(Test.class)) {
--- End diff --

I think it is a little bit hard to check whether the usage is in src/test 
path. I can only get which invoked this method[ which modified by a 
@VisibleForTesting].  Like the following picture. I can know which invoke the 
```generateTablesForClass``` method. which are in usages field. But It seems I 
can not figure out these classes whether come from ```src/path```.  Only I can 
get belongs to it's target class path. Like: 
```file:/D:/projects/flink/flink-core/target/classes/org/apache/flink/configuration/ConfigOptionsDocGenerator.class```.
 That is to say, I can find where java class is loaded from. But I can not get 
it's source code comes from.


![test](https://user-images.githubusercontent.com/6520673/31723324-1d529142-b451-11e7-9f85-a61d859702b0.png)



> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16209322#comment-16209322
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145412067
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
+   
.useParallelExecutor(Runtime.getRuntime().availableProcessors())
+   .addUrls(ClasspathHelper.forPackage("org.apache.flink"))
+   .addScanners(new MemberUsageScanner(),
+   new MethodAnnotationsScanner()));
+
+   Set methods = 
reflections.getMethodsAnnotatedWith(VisibleForTesting.class);
+
+   for (Method method : methods) {
+   Set usages = reflections.getMethodUsage(method);
+   for (Member member : usages) {
+   if (member instanceof Method) {
+   Method methodHopeWithTestAnnotation = 
(Method) member;
--- End diff --

Yeah. Thanks! I think it is an awesome name really!


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16209285#comment-16209285
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145404978
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
--- End diff --

OKay. I will.


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16209277#comment-16209277
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145403730
  
--- Diff: 
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
 ---
@@ -439,7 +439,7 @@ public CheckpointingMode getCheckpointingMode() {
 * from operations on {@link 
org.apache.flink.streaming.api.datastream.KeyedStream}) is maintained
 * (heap, managed memory, externally), and where state 
snapshots/checkpoints are stored, both for
 * the key/value state, and for checkpointed functions (implementing 
the interface
-* {@link org.apache.flink.streaming.api.checkpoint.Checkpointed}).
+* {@link 
org.apache.flink.streaming.api.checkpoint.CheckpointedFunction}).
--- End diff --

I have changed it to ```ListCheckpointed``` class instead.


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16209260#comment-16209260
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145401310
  
--- Diff: 
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
 ---
@@ -439,7 +439,7 @@ public CheckpointingMode getCheckpointingMode() {
 * from operations on {@link 
org.apache.flink.streaming.api.datastream.KeyedStream}) is maintained
 * (heap, managed memory, externally), and where state 
snapshots/checkpoints are stored, both for
 * the key/value state, and for checkpointed functions (implementing 
the interface
-* {@link org.apache.flink.streaming.api.checkpoint.Checkpointed}).
+* {@link 
org.apache.flink.streaming.api.checkpoint.CheckpointedFunction}).
--- End diff --

Thanks @pnowojski for review. I am appreciate it! Hmm, I am not sure if it 
exists maven plugins tools for checking the same thing. I will updated the code 
based on your suggestions. Then, watch the test what will happen. Why did I put 
it in ```manual``` package, because I found a class named 
```CheckForbiddenMethodsUsage``` might did the same stuff. So, I also did it 
like this. Do you have any good idea where it should put ? 


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16207657#comment-16207657
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user pnowojski commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145125984
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
+   
.useParallelExecutor(Runtime.getRuntime().availableProcessors())
+   .addUrls(ClasspathHelper.forPackage("org.apache.flink"))
+   .addScanners(new MemberUsageScanner(),
--- End diff --

if you fold arguments, please also fold the first one:

```
.addScanners(
new MemberUsageScanner(),
new MethodAnnotationsScanner())
```


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16207655#comment-16207655
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user pnowojski commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145125538
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
--- End diff --

Could you extract `ConfigurationBuilder` to a  local variable? Now it is 
hard to read.


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16207658#comment-16207658
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user pnowojski commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145132472
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
+   
.useParallelExecutor(Runtime.getRuntime().availableProcessors())
+   .addUrls(ClasspathHelper.forPackage("org.apache.flink"))
+   .addScanners(new MemberUsageScanner(),
+   new MethodAnnotationsScanner()));
+
+   Set methods = 
reflections.getMethodsAnnotatedWith(VisibleForTesting.class);
+
+   for (Method method : methods) {
+   Set usages = reflections.getMethodUsage(method);
+   for (Member member : usages) {
+   if (member instanceof Method) {
+   Method methodHopeWithTestAnnotation = 
(Method) member;
--- End diff --

nit: maybe rename `methodHopeWithTestAnnotation` to 
`visibleForTestingUsageScope`? Or something else?


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16207659#comment-16207659
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user pnowojski commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145126703
  
--- Diff: 
flink-tests/src/test/java/org/apache/flink/test/manual/CheckVisibleForTestingUsage.java
 ---
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.test.manual;
+
+import org.apache.flink.annotation.VisibleForTesting;
+
+import org.junit.Test;
+import org.reflections.Reflections;
+import org.reflections.scanners.MemberUsageScanner;
+import org.reflections.scanners.MethodAnnotationsScanner;
+import org.reflections.util.ClasspathHelper;
+import org.reflections.util.ConfigurationBuilder;
+
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
+import java.util.Set;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * This test check the methods are annotated with @VisibleForTesting. But 
still was called from the class
+ * which does not belong to the tests. These methods should only be called 
from tests.
+ */
+public class CheckVisibleForTestingUsage {
+
+   @Test
+   public void testCheckVisibleForTesting() throws Exception {
+   final Reflections reflections = new Reflections(new 
ConfigurationBuilder()
+   
.useParallelExecutor(Runtime.getRuntime().availableProcessors())
+   .addUrls(ClasspathHelper.forPackage("org.apache.flink"))
+   .addScanners(new MemberUsageScanner(),
+   new MethodAnnotationsScanner()));
+
+   Set methods = 
reflections.getMethodsAnnotatedWith(VisibleForTesting.class);
+
+   for (Method method : methods) {
+   Set usages = reflections.getMethodUsage(method);
+   for (Member member : usages) {
+   if (member instanceof Method) {
+   Method methodHopeWithTestAnnotation = 
(Method) member;
+   if 
(!methodHopeWithTestAnnotation.isAnnotationPresent(Test.class)) {
--- End diff --

what if the usage is in tests, but wrapped by some test class/mock? Can we 
check whether the usage is in `src/test` path? 


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16207656#comment-16207656
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

Github user pnowojski commented on a diff in the pull request:

https://github.com/apache/flink/pull/4705#discussion_r145133848
  
--- Diff: 
flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
 ---
@@ -439,7 +439,7 @@ public CheckpointingMode getCheckpointingMode() {
 * from operations on {@link 
org.apache.flink.streaming.api.datastream.KeyedStream}) is maintained
 * (heap, managed memory, externally), and where state 
snapshots/checkpoints are stored, both for
 * the key/value state, and for checkpointed functions (implementing 
the interface
-* {@link org.apache.flink.streaming.api.checkpoint.Checkpointed}).
+* {@link 
org.apache.flink.streaming.api.checkpoint.CheckpointedFunction}).
--- End diff --

> (for example implementing the interface {@link 
org.apache.flink.streaming.api.checkpoint.ListCheckpointed})

`CheckpointedFunction` is also deprecated 


> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[mingleizhang (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16178428#comment-16178428
 ] 

mingleizhang commented on FLINK-6444:
-

Hey, [~yew1eb]. I think you can just use reflections do it in a simple way. And 
I guess it is more easier than your method as it is just a insignificant 
change. Not suggest to write many codes for it. And it will take potential bug 
instead probably.

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Hai Zhou_UTC+8 (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16178254#comment-16178254
 ] 

Hai Zhou_UTC+8 commented on FLINK-6444:
---

[~StephanEwen]  I implemented a maven module "flink-spotbugs-plugin" that 
contains a *VisibleForTestingDetector* class used to detect whether 
'@VisibleForTesting' methods was used illegally.

But,I am troubled. this module needs to be deployed to a remote maven 
repository, and It dep the “flink-annotation” module, the version of 
flink-annotation is not certain. 
Can you give me some suggestions? 
Or have no choice but to use the second plan like 
*CheckForbiddenMethodsUsage.java*.

BTW, via VisibleForTestingDetector detector, I did find that many 
'@VisibleForTesting' methods are called not in the test code.

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: mingleizhang
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16176444#comment-16176444
 ] 

ASF GitHub Bot commented on FLINK-6444:
---

GitHub user zhangminglei opened a pull request:

https://github.com/apache/flink/pull/4705

[FLINK-6444] [build] Add a check that '@VisibleForTesting' methods ar…

## What is the purpose of the change

## Brief change log


## Verifying this change



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zhangminglei/flink flink-6444

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/4705.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #4705


commit 425c6eaee2d76df2df4f5d2070a33b1b4b9ddbcb
Author: zhangminglei 
Date:   2017-09-22T13:44:18Z

[FLINK-6444] [build] Add a check that '@VisibleForTesting' methods are only 
used in tests




> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: Hai Zhou_UTC+8
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Hai Zhou_UTC+8 (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16176416#comment-16176416
 ] 

Hai Zhou_UTC+8 commented on FLINK-6444:
---

I am trying the first.
Implement a "Spotbugs" Detector, and configure a "Spotbugs" rule.

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: Hai Zhou_UTC+8
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Stephan Ewen (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16174497#comment-16174497
 ] 

Stephan Ewen commented on FLINK-6444:
-

Cool, thanks for picking this up.

Would be interested to hear what approach you want to take.

Spontaneously, I could think of two ways to approach that
  - See if there is a way to define a "Spotbugs" rule
  - Use the Reflections library to check and enforce that. We use it in a 
similar way to make sure that no string construction happens without explicit 
charset. See for example here: 
https://github.com/apache/flink/blob/9bd491e05120915cbde36d4452e3982fe5d0975f/flink-tests/src/test/java/org/apache/flink/test/manual/CheckForbiddenMethodsUsage.java

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>Assignee: Hai Zhou_UTC+8
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Hai Zhou (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16161119#comment-16161119
 ] 

Hai Zhou commented on FLINK-6444:
-

I already have an idea.
When I am not busy with work;), I will go to fix it.

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[mingleizhang (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16160925#comment-16160925
 ] 

mingleizhang commented on FLINK-6444:
-

Hello, [~StephanEwen] . I would like work on this. But I need check it with you 
with some points in order to have a better understanding what you were trying 
to say. What you mean is when I type mvn clean install -DskipTests, then those 
method which has @VisibleForTesting is still called from non-tests. So, we 
should do something which can prevent it from this. I am correct ?

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (FLINK-6444) Add a check that '@VisibleForTesting' methods are only used in tests

[Aljoscha Krettek (JIRA)]

[ 
https://issues.apache.org/jira/browse/FLINK-6444?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15996709#comment-15996709
 ] 

Aljoscha Krettek commented on FLINK-6444:
-

+1

> Add a check that '@VisibleForTesting' methods are only used in tests
> 
>
> Key: FLINK-6444
> URL: https://issues.apache.org/jira/browse/FLINK-6444
> Project: Flink
>  Issue Type: Improvement
>  Components: Build System
>Reporter: Stephan Ewen
>
> Some methods are annotated with {{@VisibleForTesting}}. These methods should 
> only be called from tests.
> This is currently not enforced / checked during the build. We should add such 
> a check.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)