fix Execution order of ci_check_unittest, and add it to Linux_py35 (#21640)

paddle/scripts/paddle_build.sh

@@ -39,7 +39,8 @@ function print_usage() {
     ${BLUE}dockerfile${NONE}: generate paddle release dockerfile
     ${BLUE}fluid_inference_lib${NONE}: deploy fluid inference library
     ${BLUE}check_style${NONE}: run code style check
-    ${BLUE}cicheck${NONE}: run CI tasks
+    ${BLUE}cicheck${NONE}: run CI tasks on Linux
+    ${BLUE}maccheck${NONE}: run CI tasks on Mac
     "
 }
 
@@ -77,7 +78,7 @@ function cmake_base() {
                 PYTHON_FLAGS="-DPYTHON_EXECUTABLE:FILEPATH=/Library/Frameworks/Python.framework/Versions/2.7/bin/python2.7
             -DPYTHON_INCLUDE_DIR:PATH=/Library/Frameworks/Python.framework/Versions/2.7/include/python2.7
             -DPYTHON_LIBRARY:FILEPATH=/Library/Frameworks/Python.framework/Versions/2.7/lib/libpython2.7.dylib"
-            pip install --user -r ${PADDLE_ROOT}/python/requirements.txt
+                pip install --user -r ${PADDLE_ROOT}/python/requirements.txt
             else
                 exit 1
             fi
@@ -468,7 +469,7 @@ function generate_upstream_develop_api_spec() {
     build $2
     generate_api_spec "$1" "DEV"
     git checkout $cur_branch
-    git branch -D develop_base_pr 
+    git branch -D develop_base_pr
     ENABLE_MAKE_CLEAN="OFF"
 }
 
@@ -511,54 +512,69 @@ function generate_api_spec() {
     deactivate
 }
 
-function check_change_of_unittest() {
-    fetch_upstream_develop_if_not_exist
-    cur_branch=`git branch | grep \* | cut -d ' ' -f2` 
-    git checkout -b develop_base_pr upstream/$BRANCH
-    cmake_gen $1
-    generate_unittest_spec "DEV"
-    git checkout $cur_branch
-    git branch -D develop_base_pr
-    rm -rf ${PADDLE_ROOT}/build
-    cmake_gen $1
-    generate_unittest_spec "PR"
-
-    # approval_user_list: XiaoguangHu01 46782768,luotao1 6836917,phlrain 43953930,lanxianghit 47554610, zhouwei25 52485244
+function check_approvals_of_unittest() {
+    # approval_user_list: XiaoguangHu01 46782768,luotao1 6836917,phlrain 43953930,lanxianghit 47554610, zhouwei25 52485244, kolinwei 22165420
     approval_line=`curl -H "Authorization: token ${GITHUB_API_TOKEN}" https://api.github.com/repos/PaddlePaddle/Paddle/pulls/${GIT_PR_ID}/reviews?per_page=10000`
-    tests_spec_diff=`python ${PADDLE_ROOT}/tools/diff_unittest.py ${PADDLE_ROOT}/paddle/fluid/UNITTEST_DEV.spec  ${PADDLE_ROOT}/paddle/fluid/UNITTEST_PR.spec`
-    if [ "$test_spec_diff" != "" ]; then
-        APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 46782768 6836917 43953930 47554610`
+    check_times=$1
+    if [ $1 == 1 ]; then
+        APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 22165420 52485244 6836917`
         echo "current pr ${GIT_PR_ID} got approvals: ${APPROVALS}"
-        if [ "${APPROVALS}" == "FALSE" ]; then
-            echo "****************"
-            echo -e "You must have one RD (luotao1 or XiaoguangHu01 or phlrain or lanxianghit or zhouwei25) approval for the deletion of unit tests.\n"
-            echo "There are one approved errors."
-            echo "****************"
-            exit 1
+        if [ "${APPROVALS}" == "TRUE" ]; then
+            echo "==================================="
+            echo -e "\n current pr ${GIT_PR_ID} has got approvals. So, Pass CI directly!\n"
+            echo "==================================="
+            exit 0
+        fi
+    elif [ $1 == 2 ]; then
+        unittest_spec_diff=`python ${PADDLE_ROOT}/tools/diff_unittest.py ${PADDLE_ROOT}/paddle/fluid/UNITTEST_DEV.spec ${PADDLE_ROOT}/paddle/fluid/UNITTEST_PR.spec`
+        if [ "$unittest_spec_diff" != "" ]; then
+            APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 22165420 52485244 6836917`
+            echo "current pr ${GIT_PR_ID} got approvals: ${APPROVALS}"
+            if [ "${APPROVALS}" == "FALSE" ]; then
+                echo "************************************"
+                echo -e "It is forbidden to disable or delete the unit-test.\n"
+                echo -e "If you must delete it temporarily, please add it to[https://github.com/PaddlePaddle/Paddle/wiki/Temporarily-disabled-Unit-Test]."
+                echo -e "Then you must have one RD (kolinwei(recommended) or zhouwei25 or luotao1) approval for the deletion of unit-test. \n"
+                echo -e "If you have any problems about deleting unit-test, please read the specification [https://github.com/PaddlePaddle/Paddle/wiki/Deleting-unit-test-is-forbidden]. \n"
+                echo -e "Following unit-tests are deleted in this PR: \n ${unittest_spec_diff} \n"
+                echo "************************************"
+                exit 1
+            fi
         fi
     fi
-    ENABLE_MAKE_CLEAN="OFF"
+}
+
+function check_change_of_unittest() {
+    set +x
+    generate_unittest_spec "PR"
+    fetch_upstream_develop_if_not_exist
+    git fetch upstream
+    git reset --hard upstream/$BRANCH
+    cmake_gen $1
+    generate_unittest_spec "DEV"
+    check_approvals_of_unittest 2
+    set +x
 }
 
 function generate_unittest_spec() {
     spec_kind=$1
     if [ "$spec_kind" == "DEV" ]; then
         cat <<EOF
-        ============================================================
-        Complete cmake first time to get number of unit tests in develop.
-        ============================================================
+        ============================================
+        Generate unit tests.spec of develop.
+        ============================================
 EOF
     elif [ "$spec_kind" == "PR" ]; then
         cat <<EOF
-        ============================================================
-        Complete cmake second time to get number of unit tests in this PR.
-        ============================================================
+        ============================================
+        Generate unit tests.spec of this PR.
+        ============================================
 EOF
     else
         echo "Not supported $1"
         exit 1
     fi
-    spec_path=${PADDLE_ROOT}/paddle/fluid/UNITTEST_${spec_kind}.spec 
+    spec_path=${PADDLE_ROOT}/paddle/fluid/UNITTEST_${spec_kind}.spec
     ctest -N | awk -F ':' '{print $2}' | sed '/^$/d' | sed '$d' > ${spec_path}
 }
 
@@ -1136,10 +1152,12 @@ function main() {
         check_style
         ;;
       cicheck)
+        check_approvals_of_unittest 1
         cmake_gen ${PYTHON_ABI:-""}
         build ${parallel_number}
         enable_unused_var_check
         parallel_test
+        check_change_of_unittest ${PYTHON_ABI:-""}
         ;;
       cicheck_brpc)
         cmake_gen ${PYTHON_ABI:-""}
@@ -1168,9 +1186,11 @@ function main() {
         run_mac_test ${PYTHON_ABI:-""} ${PROC_RUN:-1}
         ;;
       maccheck_py35)
-        check_change_of_unittest ${PYTHON_ABI:-""}
+        check_approvals_of_unittest 1
+        cmake_gen ${PYTHON_ABI:-""}
         build_mac
         run_mac_test ${PYTHON_ABI:-""} ${PROC_RUN:-1}
+        check_change_of_unittest ${PYTHON_ABI:-""}
         ;;
       macbuild)
         cmake_gen ${PYTHON_ABI:-""}

tools/check_api_approvals.sh

@@ -101,7 +101,7 @@ for API_FILE in ${API_FILES[*]}; do
           echo_line="You must have one RD (gongweibao or seiriosPlus) approval for the paddle/fluid/operators/distributed/send_recv.proto.in, which manages the environment variables.\n"
           check_approval 1 10721757 5442383
       elif [ "${API_FILE}" == "paddle/fluid/framework/unused_var_check.cc" ];then
-          echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.cc, which manages the white list of operators that have unused input variables. Before change the white list, please read the spicification [https://github.com/PaddlePaddle/Paddle/wiki/OP-Should-Not-Have-Unused-Input] and try to refine code first. \n"
+          echo_line="You must have one RD (zhiqiu (Recommend) , sneaxiy or luotao1) approval for the paddle/fluid/framework/unused_var_check.cc, which manages the white list of operators that have unused input variables. Before change the white list, please read the specification [https://github.com/PaddlePaddle/Paddle/wiki/OP-Should-Not-Have-Unused-Input] and try to refine code first. \n"
           check_approval 1 6888866 32832641 6836917
       else
           echo_line="You must have one RD (XiaoguangHu01,Xreki,luotao1,sneaxiy) approval for ${API_FILE}, which manages the underlying code for fluid.\n"

tools/diff_unittest.py

@@ -18,7 +18,7 @@ diffs = []
 for each_diff in result:
     if each_diff[0] == '-':  # delete unit test is not allowed
         error = True
-        diffs.append(each_diff)
+        diffs.append(each_diff[2])
 '''
 If you delete the unit test, such as commenting it out, 
 please ask for approval of one RD below for passing CI:
@@ -26,6 +26,5 @@ please ask for approval of one RD below for passing CI:
     - XiaoguangHu01 or luotao1 or phlrain or lanxianghit or zhouwei25
 '''
 if error:
-    print('Deleted Unit test is: ')
     for each_diff in diffs:
         print(each_diff)