From 1eae63b997a7a54b38ee186a4f6afb0974aabe9b Mon Sep 17 00:00:00 2001
From: Shenghang Tsai <jackalcooper@gmail.com>
Date: Fri, 16 Jul 2021 23:19:06 +0800
Subject: [PATCH] Prevent adding subdir in python/test (#5514)

* prevent adding subdir in python/test

* refine

* add ci

* refactor

* refine

* refine

* add more timeout-minutes: 45

Co-authored-by: oneflow-ci-bot <69100618+oneflow-ci-bot@users.noreply.github.com>
---
 .github/workflows/test.yml                    | 33 ++++++-
 .../test/demos/eager_nccl_all_reduce_demo.py  | 40 --------
 .../python/test/deprecated/constant_scalar.py | 26 -----
 .../deprecated/enable_all_reduce_group.py     | 78 ---------------
 .../python/test/deprecated/variable_scope.py  | 98 -------------------
 .../ops/{generator => }/test_generator.py     |  0
 tools/check_src.py                            | 54 ++++++++++
 7 files changed, 85 insertions(+), 244 deletions(-)
 delete mode 100644 oneflow/python/test/demos/eager_nccl_all_reduce_demo.py
 delete mode 100644 oneflow/python/test/deprecated/constant_scalar.py
 delete mode 100644 oneflow/python/test/deprecated/enable_all_reduce_group.py
 delete mode 100644 oneflow/python/test/deprecated/variable_scope.py
 rename oneflow/python/test/ops/{generator => }/test_generator.py (100%)
 create mode 100644 tools/check_src.py

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index f7110ace7..7f5ec0909 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -91,6 +91,9 @@ jobs:
         if: ${{ failure() }}
         run: |
           exit 1
+      - name: Check source code (prevent creating files at wrong places)
+        run: |
+          python3 tools/check_src.py
 
   static_analysis_with_clang:
     name: Static analysis with clang
@@ -133,7 +136,7 @@ jobs:
             -DBUILD_TESTING=ON \
             -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
           cd ..
-          ./run-clang-tidy.py -clang-tidy-binary ./clang-tidy -p build -quiet 
+          ./run-clang-tidy.py -clang-tidy-binary ./clang-tidy -p build -quiet
 
   wait_for_gpu_slot:
     name: Wait for GPU slots
@@ -251,9 +254,9 @@ jobs:
           ref: ${{ github.event.pull_request.head.sha }}
         if: env.is_built != '1'
       - name: Build OneFlow
+        timeout-minutes: 45
         if: env.is_built != '1'
         uses: ./.github/actions/whl
-        timeout-minutes: 45
         with:
           tmp_dir: ${ci_tmp_dir}
           extra_flags: ${{ matrix.extra_flags }}
@@ -261,6 +264,7 @@ jobs:
           extra_docker_args: $extra_docker_args
           python_version: ${{ matrix.python_version }}
       - name: Custom Op test (run by oneflow build docker)
+        timeout-minutes: 45
         if: matrix.test_suite == 'cpu' && env.is_built != '1'
         run: |
           set -x
@@ -423,6 +427,7 @@ jobs:
         run: |
           bash docker/ci/test/build.sh
       - name: Exe test
+        timeout-minutes: 45
         if: contains(fromJson('["cuda", "cpu"]'), matrix.test_suite)
         run: |
           set -x
@@ -431,6 +436,7 @@ jobs:
             ${image_name} \
             ${bin_dir}/oneflow_testexe
       - name: Build documentation
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda'
         run: |
           set -x
@@ -438,18 +444,21 @@ jobs:
             ${image_name} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/build_docs.sh"
       - name: Op test (distributed, 1st try)
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda'
         continue-on-error: true
         id: distributed_try_1
         run: |
           python3 ci/test/distributed_run.py --bash_script=ci/test/2node_op_test.sh --custom_img_tag=${{ env.image_name }} --oneflow_wheel_path=${{ env.wheelhouse_dir }} --oneflow_wheel_python_version=3.6
       - name: Op test (distributed, 2nd try)
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda' && steps.distributed_try_1.outcome=='failure'
         continue-on-error: true
         id: distributed_try_2
         run: |
           python3 ci/test/distributed_run.py --bash_script=ci/test/2node_op_test.sh --custom_img_tag=${{ env.image_name }} --oneflow_wheel_path=${{ env.wheelhouse_dir }} --oneflow_wheel_python_version=3.6
       - name: Op test (distributed, 3rd try)
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda' && steps.distributed_try_2.outcome=='failure'
         continue-on-error: false
         id: distributed_try_3
@@ -472,6 +481,7 @@ jobs:
             ${{ env.extra_docker_args }} ${{ env.pip_cache_docker_args }} \
             ${image_name} bash ci/test/print_stack_from_core.sh python3 distributed-tmp
       - name: Doctest
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda'
         run: |
           set -x
@@ -480,6 +490,7 @@ jobs:
             ${image_name} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/doctest.sh"
       - name: Dry run test (run without runtime)
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda'
         run: |
           set -x
@@ -508,6 +519,7 @@ jobs:
           wget ${{ env.image_url }}
           docker load -i $(basename "${{ env.image_url }}")
       - name: Module API test
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda_new_interface'
         run: |
           docker run \
@@ -515,7 +527,17 @@ jobs:
             -e ONEFLOW_TEST_DIR=$PWD/oneflow/python/test/modules \
             ${{ env.image_tag }} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/generic_test.sh"
+      - name: Dataloader API test
+        timeout-minutes: 45
+        if: matrix.test_suite == 'cuda_new_interface'
+        run: |
+          docker run \
+            ${{ env.extra_docker_args }} ${{ env.pip_cache_docker_args }} \
+            -e ONEFLOW_TEST_DIR=$PWD/oneflow/python/test/dataloader \
+            ${{ env.image_tag }} \
+            bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/generic_test.sh"
       - name: Tensor API test
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda_new_interface'
         run: |
           docker run \
@@ -541,6 +563,7 @@ jobs:
             ${image_name} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/1node_op_test.sh"
       - name: Model test
+        timeout-minutes: 45
         if: matrix.test_suite == 'cpu' || matrix.test_suite == 'cuda'
         run: |
           set -x
@@ -549,6 +572,7 @@ jobs:
             ${image_name} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/1node_model_test.sh"
       - name: Model serve test
+        timeout-minutes: 45
         id: model_serve_test
         if: matrix.test_suite == 'cuda'
         run: |
@@ -564,6 +588,7 @@ jobs:
           docker run ${{ env.extra_docker_args }} ${{ env.pip_cache_docker_args }} \
             ${image_name} bash ci/test/print_stack_from_core.sh python3 serving-tmp
       - name: Benchmark (mainly for backward compatibility)
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda'
         run: |
           set -x
@@ -571,6 +596,7 @@ jobs:
             ${image_name} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/1node_benchmark_test.sh"
       - name: Benchmark FP16 (mainly for backward compatibility)
+        timeout-minutes: 45
         if: matrix.test_suite == 'cuda'
         run: |
           set -x
@@ -578,6 +604,7 @@ jobs:
             ${image_name} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/1node_benchmark_test_fp16.sh"
       - name: XLA Test
+        timeout-minutes: 45
         if: contains(fromJson('["xla", "xla_cpu"]'), matrix.test_suite) && env.is_built != '1'
         run: |
           set -x
@@ -585,11 +612,13 @@ jobs:
             ${image_name} \
             bash -c "python3 -m pip config set global.index-url ${{ env.pip_index_mirror }} && bash ci/test/try_install.sh && bash ci/test/test_xla.sh"
       - name: Query system status
+        timeout-minutes: 45
         if: ${{ failure() }}
         run: |
           nvidia-smi
           docker ps
       - name: Remove container
+        timeout-minutes: 45
         if: always()
         run: |
           docker rm -f ${container_name} || true
diff --git a/oneflow/python/test/demos/eager_nccl_all_reduce_demo.py b/oneflow/python/test/demos/eager_nccl_all_reduce_demo.py
deleted file mode 100644
index aeaf83d53..000000000
--- a/oneflow/python/test/demos/eager_nccl_all_reduce_demo.py
+++ /dev/null
@@ -1,40 +0,0 @@
-"""
-Copyright 2020 The OneFlow Authors. All rights reserved.
-
-Licensed 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.
-"""
-import unittest
-import oneflow as flow
-import numpy as np
-import sys
-import oneflow.typing as oft
-
-flow.config.gpu_device_num(4)
-
-func_config = flow.FunctionConfig()
-func_config.default_data_type(flow.float)
-func_config.default_logical_view(flow.scope.consistent_view())
-
-if __name__ == "__main__":
-
-    @flow.global_function(function_config=func_config)
-    def test_job(x: oft.Numpy.Placeholder((10000,), dtype=flow.float)):
-        return flow.eager_nccl_all_reduce(
-            x, parallel_conf="""  device_tag: "gpu", device_name: "0:0-3" """,
-        )
-
-    for _ in range(10):
-        x = np.random.rand(10000).astype(np.float32)
-        y = test_job(x).get()
-        print(x)
-        print(y)
diff --git a/oneflow/python/test/deprecated/constant_scalar.py b/oneflow/python/test/deprecated/constant_scalar.py
deleted file mode 100644
index d4b0123ff..000000000
--- a/oneflow/python/test/deprecated/constant_scalar.py
+++ /dev/null
@@ -1,26 +0,0 @@
-"""
-Copyright 2020 The OneFlow Authors. All rights reserved.
-
-Licensed 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.
-"""
-import oneflow as flow
-
-flow.config.gpu_device_num(1)
-
-
-@flow.global_function()
-def ConstantScalarJob():
-    return dlnet.ConstantScalar(3.14)
-
-
-print(ConstantScalarJob().get())
diff --git a/oneflow/python/test/deprecated/enable_all_reduce_group.py b/oneflow/python/test/deprecated/enable_all_reduce_group.py
deleted file mode 100644
index f41f4be9c..000000000
--- a/oneflow/python/test/deprecated/enable_all_reduce_group.py
+++ /dev/null
@@ -1,78 +0,0 @@
-"""
-Copyright 2020 The OneFlow Authors. All rights reserved.
-
-Licensed 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.
-"""
-import numpy as np
-import oneflow as flow
-import oneflow.core.common.data_type_pb2 as data_type_conf_util
-import oneflow.core.operator.op_conf_pb2 as op_conf_util
-
-flow.config.gpu_device_num(2)
-
-
-def UpdateVariable(x, scope_name):
-    with flow.variable_scope(scope_name):
-        w = flow.get_variable(
-            name=scope_name + "-w",
-            shape=(10,),
-            dtype=flow.float,
-            initializer=flow.constant_initializer(value=1.0),
-        )
-        c = flow.nn.bias_add(x, w)
-        # return flow.keras.activations.tanh(c)
-        loss = flow.math.reduce_mean(c, name="loss_op")
-        flow.losses.add_loss(loss)
-        return loss
-
-
-input_blob_def = flow.FixedTensorDef((2, 10), dtype=flow.float)
-func_config = flow.FunctionConfig()
-func_config.train.primary_lr(0.01)
-func_config.train.model_update_conf(dict(naive_conf={}))
-func_config.enable_all_reduce_group(True)
-
-
-@flow.global_function(func_config)
-def OneDeviceUpdateVariable(x=input_blob_def):
-    with flow.scope.placement("gpu", "0:0"):
-        return UpdateVariable(x, "one-device")
-
-
-@flow.global_function(func_config)
-def TwoDeviceUpdateVariable(x=input_blob_def):
-    with flow.scope.placement("gpu", "0:0-1"):
-        return UpdateVariable(x, "two-device")
-
-
-func_config.enable_all_reduce_group(True)
-
-
-@flow.global_function(func_config)
-def DisableAllReduceGroupUpdateVariable(x=input_blob_def):
-    with flow.scope.placement("gpu", "0:0-1"):
-        return UpdateVariable(x, "disable-all-reduce-group")
-
-
-print "-------- one device --------"
-for i in range(10):
-    x = OneDeviceUpdateVariable(np.ones((2, 10), dtype=np.float32)).get()
-    print (x)
-print "-------- two device --------"
-for i in range(10):
-    x = TwoDeviceUpdateVariable(np.ones((2, 10), dtype=np.float32)).get()
-    print (x)
-print "-------- disable all reduce group --------"
-for i in range(10):
-    x = DisableAllReduceGroupUpdateVariable(np.ones((2, 10), dtype=np.float32)).get()
-    print (x)
diff --git a/oneflow/python/test/deprecated/variable_scope.py b/oneflow/python/test/deprecated/variable_scope.py
deleted file mode 100644
index 90dd14e02..000000000
--- a/oneflow/python/test/deprecated/variable_scope.py
+++ /dev/null
@@ -1,98 +0,0 @@
-"""
-Copyright 2020 The OneFlow Authors. All rights reserved.
-
-Licensed 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.
-"""
-import numpy as np
-import oneflow as of
-
-
-@flow.global_function
-def variable_scope_test_job_1(a=of.FixedTensorDef((1, 3, 6, 6))):
-    with of.scope.namespace("job1_scope1"):
-        convw = of.get_variable(
-            "conv_weight",
-            shape=(5, 3, 3, 3),
-            dtype=a.dtype,
-            initializer=of.random_uniform_initializer(),
-            trainable=True,
-        )
-        conv = of.nn.conv2d(a, convw, 1, "SAME", None, "NCHW", name="conv")
-
-        with of.scope.namespace("job1_scope2"):
-            fcw = of.get_variable(
-                "fc_weight",
-                shape=(180, 10),
-                dtype=a.dtype,
-                initializer=of.random_uniform_initializer(),
-                trainable=True,
-            )
-            fc = of.matmul(of.reshape(conv, (conv.shape[0], -1)), fcw, name="fc")
-            fcb = of.get_variable(
-                "fc_bias",
-                shape=(10,),
-                dtype=a.dtype,
-                initializer=of.constant_initializer(1.0),
-                trainable=True,
-            )
-            fc_bias = of.nn.bias_add(fc, fcb)
-
-        fcw2 = of.get_variable(
-            "fc2_weight",
-            shape=(10, 20),
-            dtype=a.dtype,
-            initializer=of.random_uniform_initializer(),
-            trainable=True,
-        )
-        fc2 = of.matmul(fc_bias, fcw2, name="fc2")
-
-    print("conv_weight op name: ", convw.op_name)
-    print("conv op name: ", conv.op_name)
-    print("fc_weight op name: ", fcw.op_name)
-    print("fc_bias op name: ", fcb.op_name)
-    print("fc op name: ", fc.op_name)
-    print("fc2_weight op name: ", fcw2.op_name)
-    print("fc2 op name: ", fc2.op_name)
-
-    return fc2
-
-
-@flow.global_function
-def variable_scope_test_job_2(a=of.FixedTensorDef((2, 5))):
-    with of.scope.namespace("job2_scope1"):
-        indices = of.get_variable(
-            "gather_inds",
-            shape=(2,),
-            dtype=of.int32,
-            initializer=of.constant_initializer(1),
-            trainable=False,
-        )
-        output = of.gather(a, indices, axis=1)
-
-    print("indices op name: ", indices.op_name)
-    print("gather op name: ", output.op_name)
-    return output
-
-
-a1 = np.random.rand(1, 3, 6, 6).astype(np.float32)
-a2 = np.arange(10, dtype=np.float32).reshape(2, 5)
-ret1 = variable_scope_test_job_1.run(a1).get()
-ret2 = variable_scope_test_job_2(a2).get()
-
-print("Job1 result: ")
-print(ret1)
-print("shape: ", ret1.shape)
-print("\n")
-print("Job2 result: ")
-print(ret2)
-print("shape: ", ret2.shape)
diff --git a/oneflow/python/test/ops/generator/test_generator.py b/oneflow/python/test/ops/test_generator.py
similarity index 100%
rename from oneflow/python/test/ops/generator/test_generator.py
rename to oneflow/python/test/ops/test_generator.py
diff --git a/tools/check_src.py b/tools/check_src.py
new file mode 100644
index 000000000..82bf5bc82
--- /dev/null
+++ b/tools/check_src.py
@@ -0,0 +1,54 @@
+import os
+
+this_file = os.path.dirname(os.path.abspath(__file__))
+src_root = os.path.join(this_file, "..")
+src_root = os.path.abspath(src_root)
+
+
+def check_unwanted_test_scripts(python_test_dir=None, allowed=None):
+    python_test_dir = os.path.abspath(python_test_dir)
+
+    allowed_full = [
+        os.path.relpath(os.path.join(python_test_dir, a), src_root) for a in allowed
+    ]
+    for (dirpath, dirnames, filenames) in os.walk(src_root):
+        if python_test_dir in dirpath and "__pycache__" not in dirpath:
+            rel_to_python_test = os.path.relpath(dirpath, python_test_dir)
+            rel_to_src_root = os.path.relpath(dirpath, src_root)
+            print(f"checking: {rel_to_src_root}")
+            if (
+                rel_to_python_test not in allowed
+                and rel_to_python_test != "."
+                and "custom_ops" not in rel_to_python_test
+            ):
+                if filenames == []:
+                    raise ValueError(f"delete this directory: {rel_to_src_root}")
+                else:
+                    filenames_full = [
+                        os.path.relpath(os.path.join(dirpath, a), src_root)
+                        for a in filenames
+                    ]
+                    raise ValueError(
+                        f"""move these files:
+    {filenames_full}
+    inside one of these directories:
+    {allowed_full},
+    and delete this directory: {rel_to_src_root}"""
+                    )
+
+
+check_unwanted_test_scripts(
+    python_test_dir=os.path.join(src_root, "oneflow/python/test"),
+    allowed=[
+        "customized",
+        "custom_ops",
+        "dataloader",
+        "graph",
+        "models",
+        "modules",
+        "ops",
+        "serving",
+        "tensor",
+        "xrt",
+    ],
+)
-- 
GitLab