From 013225bb68fc117b9e63efa41c329da76d1294e4 Mon Sep 17 00:00:00 2001 From: zhouwei25 <52485244+zhouwei25@users.noreply.github.com> Date: Fri, 20 Dec 2019 18:33:47 +0800 Subject: [PATCH] fix Execution order of ci_check_unittest, and add it to Linux_py35 (#21640) --- paddle/scripts/paddle_build.sh | 88 +++++++++++++++++++++------------- tools/check_api_approvals.sh | 2 +- tools/diff_unittest.py | 3 +- 3 files changed, 56 insertions(+), 37 deletions(-) diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index 5bf3e4d5b0b..02f48483008 100755 --- a/paddle/scripts/paddle_build.sh +++ b/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 < ${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:-""} diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index 0d89c6e74bb..859f254494f 100644 --- a/tools/check_api_approvals.sh +++ b/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" diff --git a/tools/diff_unittest.py b/tools/diff_unittest.py index 863e2beabfe..5578f4f9378 100644 --- a/tools/diff_unittest.py +++ b/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) -- GitLab