fix fail test with cover flag #5117
Conversation
scripts/run-unit-tests.sh
Outdated
| go test "${flags[@]}" "${race_flags[@]}" -tags "$GO_TAGS" "${parallel[@]}" -short -timeout=20m -skip=NoCover | ||
| fi | ||
|
|
||
| local -a no_coverage | ||
| while IFS= read -r pkg; do no_coverage+=("$pkg"); done < <(no_coverage_test_packages "$@") | ||
| if [ "${#no_coverage[@]}" -ne 0 ]; then | ||
| echo "test with no coverage" | ||
| unset flags[0] | ||
| flags=("${flags[@]}") | ||
| go test "${flags[@]}" "${race_flags[@]}" -tags "$GO_TAGS" "${no_coverage[@]}" -short -timeout=20m -run=NoCover |
There was a problem hiding this comment.
I'm fine with the approach here, but I was initially confused because I didn't see a -cover flag. Then I realized it was set in line 150 and then unset in line 181.
To make it less confusing, I'd suggest we should not hide the -cover flag inside a flags variable, only to unset it later. Could we just use -cover in the go test statements above to make it more explicit about where -cover is used and where it is not used?
Also I suspect in the future we will forget about why this was done and how it all works together, could you add some comments in the script explaining the rationale?
There was a problem hiding this comment.
Also I just noticed run_tests_with_coverage() further down in the script, I assume that will be broken, could we exclude the NoCover tests there as well?
Signed-off-by: Fedor Partanskiy <[email protected]>
Let me remind you that in go they changed the mechanism of tests with the -cover flag. Because of this we started to crash tests with plugins. A temporary solution was found.
Today our colleagues from go replied that they will not change the cover mechanism. So we can exclude dangerous tests from testing with the cover flag, but run them without the flag.
Below is my suggestion on how to do it on a permanent basis.