Add file and directory assertions #115
Conversation
shunit2
Outdated
| shunit_return=${SHUNIT_FALSE} | ||
| fi | ||
|
|
||
| unset shunit_message_ shunit_file_ |
There was a problem hiding this comment.
Could you also unset shunit_custom_message_? Same below.
shunit2
Outdated
| shunit_message_="${shunit_message_}$shunit_custom_message_" | ||
| else | ||
| shunit_file_=$1 | ||
| shunit_message_="${shunit_message_}The file '$shunit_file_' does not exist" |
There was a problem hiding this comment.
Wrap $shunit_file in {}. Same below.
shunit2
Outdated
| if command [ $# -eq 2 ]; then | ||
| shunit_custom_message_=$1 | ||
| shunit_file_=$2 | ||
| shunit_message_="${shunit_message_}$shunit_custom_message_" |
There was a problem hiding this comment.
Wrap $shunit_custom_message_ in {}. Same below.
shunit2
Outdated
| fi | ||
|
|
||
| shunit_return=${SHUNIT_TRUE} | ||
| if command [ -f "$shunit_file_" ]; then |
There was a problem hiding this comment.
Wrap $shunit_file_ in {}, and below.
shunit2
Outdated
| fi | ||
|
|
||
| shunit_return=${SHUNIT_TRUE} | ||
| if command [ -d "$shunit_directory_" ]; then |
shunit2_test_helpers
Outdated
| @@ -226,6 +226,13 @@ _th_showOutput() { | |||
| unset _th_return_ _th_stdout_ _th_stderr_ | |||
| } | |||
|
|
|||
| # Constants used to test file and directory assertions | |||
There was a problem hiding this comment.
As these are only used in the one unit test, they should be present in just that file rather than here. It makes it easier for somebody reading the code to find them. Also, it would make the tests cleaner if the constants were defined explicitly only in the test they were used in.
shunit2_test_helpers
Outdated
| @@ -226,6 +226,13 @@ _th_showOutput() { | |||
| unset _th_return_ _th_stdout_ _th_stderr_ | |||
| } | |||
|
|
|||
| # Constants used to test file and directory assertions | |||
| TH_EXISTING_DIRECTORY=$(exec 2>/dev/null;cd -- "$(dirname "$0")" || exit; unset PWD; /usr/bin/pwd || /bin/pwd || pwd) | |||
There was a problem hiding this comment.
Could you instead create an explicit directory, rather than using shell to construct one? E.g. "${SHUNIT_TMP}/this_directory_exists".
This makes the test much easier to read, and when a future somebody sees an error with your explicit directory name, they can search for that directory name in the shunit2 code and find the problem much faster.
shunit2_test_helpers
Outdated
| @@ -226,6 +226,13 @@ _th_showOutput() { | |||
| unset _th_return_ _th_stdout_ _th_stderr_ | |||
| } | |||
|
|
|||
| # Constants used to test file and directory assertions | |||
| TH_EXISTING_DIRECTORY=$(exec 2>/dev/null;cd -- "$(dirname "$0")" || exit; unset PWD; /usr/bin/pwd || /bin/pwd || pwd) | |||
| TH_EXISTING_FILE="${TH_EXISTING_DIRECTORY}/$0" | |||
There was a problem hiding this comment.
Same here. Use an explicit filename (name of your choice) inside of ${SHUNIT_TMP}. $0 is dynamic, and one cannot search for the resulting filename in the code and find it, making it more difficult to debug the code.
In general with unit tests, be as explicit as possible.
| @@ -246,6 +246,110 @@ testAssertFalse() { | |||
| th_assertFalseWithError 'too many arguments' $? "${stdoutF}" "${stderrF}" | |||
| } | |||
|
|
|||
| testAssertFileExists() { | |||
| ( assertFileExists "$TH_EXISTING_FILE" >"${stdoutF}" 2>"${stderrF}" ) | |||
There was a problem hiding this comment.
Rather than using a variable (i.e. ${TH_EXISTING_FILE}), be explicit (see comment later) about which file you are testing for. This makes the code easier to read, and easier to debug.
E.g., if you chose the name 'this_file_exists', then use exactly that filename in the test rather than a variable.
With unit tests, it is best not to "hide" info inside of variables as it makes it more difficult to read the test, and definitely more difficult to debug as one cannot search for the failed strings as easily.
|
Is it planned to implement it at some point ? |
|
I am looking forward to this change to be merged |
|
Sorry, I'm not using this anymore and I really don't have any time right know to make the requested changes. |
|
Ok let's see if I can do them |
ddidier
force-pushed
the
feature/add_filesystem_assertions
branch
from
f0918a8 to
503c736
Compare
|
I tried to fix this but a lot has changed since the last time. |
No description provided.