Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check added ut on windows #31826

Merged

Conversation

XieYunshen
Copy link
Contributor

PR types

Others

PR changes

Others

Describe

windows任务增加新增单测检查

@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

if [ -f "$PADDLE_ROOT/added_ut" ];then
added_uts=^$(awk BEGIN{RS=EOF}'{gsub(/\n/,"$|^");print}' $PADDLE_ROOT/added_ut)$
ctest -R "(${added_uts})" --output-on-failure -C Release --repeat-until-fail 3 --timeout 150;added_ut_error=$?
if [ "$added_ut_error" != 0 ];then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是不是在前面--timeout 150 设置了就可以,前面设的是150秒

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的,

@@ -492,6 +492,12 @@ setlocal enabledelayedexpansion
:: if %errorlevel% NEQ 0 exit /b 8
:: for /F %%# in ('cmd /C nvidia-smi -L ^|find "GPU" /C') do set CUDA_DEVICE_COUNT=%%#
set CUDA_DEVICE_COUNT=1
echo cmake .. -G %GENERATOR% -DCMAKE_BUILD_TYPE=Release -DWITH_AVX=%WITH_AVX% -DWITH_GPU=%WITH_GPU% -DWITH_MKL=%WITH_MKL% ^
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是不是叫win_cmake.sh更合适些

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的,我修改一下

cd $PADDLE_ROOT
echo "================================="
echo "br-ut"
cat $PADDLE_ROOT/br-ut
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

br-ut的作用是

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

br-ut是为了获取upstream/develop分支上的单测,br-ut和pr-ut对比可以得到新增单测的名称

if [ `git branch | grep 'prec_added_ut'` ];then
git branch -D 'prec_added_ut'
fi
git stash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个目的是

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是为了保存当前分支的工作进度,我去掉再测试一下有没有影响

@@ -36,6 +36,14 @@ else
disable_ut_quickly=''
fi

# check added ut
set +e
cp $PADDLE_ROOT/tools/check_added_ut.sh $PADDLE_ROOT/tools/check_added_ut_win.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个一定需要先cp吗 可以直接执行吗

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

直接执行,git会报冲突的错误,没有找到好的解决方法,只能先复制一个脚本执行,linux上也是复制了paddle_build.sh文件,然后执行cmake;
image

image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

-DWITH_INFERENCE_API_TEST=%WITH_INFERENCE_API_TEST% -DTHIRD_PARTY_PATH=%THIRD_PARTY_PATH% ^
-DINFERENCE_DEMO_INSTALL_DIR=%INFERENCE_DEMO_INSTALL_DIR% -DWITH_STATIC_LIB=%WITH_STATIC_LIB% ^
-DWITH_TENSORRT=%WITH_TENSORRT% -DTENSORRT_ROOT="%TENSORRT_ROOT%" -DMSVC_STATIC_CRT=%MSVC_STATIC_CRT% ^
-DWITH_UNITY_BUILD=%WITH_UNITY_BUILD% -DCUDA_ARCH_NAME=%CUDA_ARCH_NAME% >> %work_dir%\get_added_ut.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改一下吧

@@ -36,6 +36,14 @@ else
disable_ut_quickly=''
fi

# check added ut
set +e
cp $PADDLE_ROOT/tools/check_added_ut.sh $PADDLE_ROOT/tools/check_added_ut_win.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

zhwesky2010
zhwesky2010 previously approved these changes Mar 31, 2021
Copy link
Contributor

@zhwesky2010 zhwesky2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

ctest -R "(${added_uts})" --output-on-failure -C Release --repeat-until-fail 3;added_ut_error=$?
if [ "$added_ut_error" != 0 ];then
echo "========================================"
echo "Added UT should not exceed 15 seconds"
Copy link
Contributor

@zhwesky2010 zhwesky2010 Mar 31, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个描述是不是不准确,因为看起来是新增单测失败了,不是超过15S挂了,然后上面还要加个TIMEOUT的新单测限定时间,因为是串行跑的,我建议和Linux限定一样为15S

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的描述我改一下吧,加上时间限制的话,我觉得可以适当放宽一下,比如新增单测限制时间为30S,
Linux上的描述我一块改一下,增加"新增单测必须通过额外的三次执行检测"的描述;

@zhwesky2010 zhwesky2010 self-requested a review March 31, 2021 02:59
zhwesky2010
zhwesky2010 previously approved these changes Apr 6, 2021
@XieYunshen XieYunshen merged commit e09f4db into PaddlePaddle:develop Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants