test_cmd.sh: Correct test for are-images-different #1902
Conversation
are_images_equal exits with status 1 when the images are different, and exits with status 2 when there is an error, such as file not found. When we want to test if the images are different, we should test specifically for status 1 rather than any nonzero (failure) status.
| "${ARE_IMAGES_EQUAL}" "${INPUT_UTF8_Y4M}" "${DECODED_UTF8_FILE}" 0 || RET=$? | ||
| if [ ${RET} -ne 1 ]; then | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Line 73 probably should receive the same treatment.
I discovered this issue when I tested test_cmd.sh with a Windows are_images_equal executable that didn't support UTF-8. test_cmd.sh still passed because exit status 2 (for "file not found") was treated as "images are different".
There was a problem hiding this comment.
Why not directly
if [ $("${ARE_IMAGES_EQUAL}" "${INPUT_UTF8_Y4M}" "${DECODED_UTF8_FILE}" 0) -ne 1 ]; then
exit 1
fiThere was a problem hiding this comment.
I also tried that, but found that $(command) is the standard output of the command, not its exit status.
Because this script does "set -x" and this command is expected to fail, the script needs to test this command, otherwise the command's failure will cause the script to exit. This is why I use the awkward command || RET=$? solution. (I found it in a Stack Overflow article.) I hope there is a cleaner solution.
| "${ARE_IMAGES_EQUAL}" "${INPUT_UTF8_Y4M}" "${DECODED_UTF8_FILE}" 0 || RET=$? | ||
| if [ ${RET} -ne 1 ]; then | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Why not directly
if [ $("${ARE_IMAGES_EQUAL}" "${INPUT_UTF8_Y4M}" "${DECODED_UTF8_FILE}" 0) -ne 1 ]; then
exit 1
fi
are_images_equal exits with status 1 when the images are different, and exits with status 2 when there is an error, such as file not found.
When we want to test if the images are different, we should test specifically for status 1 rather than any nonzero (failure) status.