Unexpected errors on files without tests
mtmiller opened this issue · 6 comments
Both test2 and runtests2 raise an error on certain types of files that have no tests in them. I hit this error when trying to use runtests2 on Pythonic, and noticing the error when it hits .cc files without tests.
Simplest reproducible example is with empty files testpkg/inst/foo.m and testpkg/src/foo.cc.
>> test2 testpkg/inst/foo.m
error: value on right hand side of assignment is undefined
error: called from
test2 at line 202 column 8
>> test2 testpkg/src/foo.cc
error: value on right hand side of assignment is undefined
error: called from
test2 at line 202 column 8
With runtests2, the situation is slightly different, I suppose depending on the file name?
>> runtests2 testpkg/inst
Summary:
GNU Octave Version: 5.1.0 (hg id: d05d6eebde10)
Tests run on galago.mtmxr.com (Unix) at 17-May-2019 11:19:11
Test execution time: 00:00:00
PASS 0
FAIL 0
>> runtests2 testpkg/src
error: if: undefined value used in conditional expression
error: called from
run_tests at line 341 column 11
runtests2 at line 114 column 9
Welp, that's a bug. I think in all these cases, it should produce a PASS result with 0/0 tests passed. (Because a PASS result is by definition 0 "real" test failures.)
I suppose one could argue that an empty .cc file is not a valid C/C++ source file, so I suppose that could be an error, but I don't think the test code should be looking that closely at them; it should just try to extract test blocks.
I think in all these cases, it should produce a PASS result with 0/0 tests passed. (Because a PASS result is by definition 0 "real" test failures.)
I would think just skip empty files or files that have no test content, right? At least for MultiBistRunner, files should just be silently skipped. For BistRunner, maybe you want to show every file that was explicitly tested.
I suppose one could argue that an empty
.ccfile is not a valid C/C++ source file, so I suppose that could be an error, but I don't think the test code should be looking that closely at them; it should just try to extract test blocks.
Yeah, definitely not looking for an error on the C++ contents of the file, way out of scope. Just skip over files that have no Octave test content.
I would think just skip empty files or files that have no test content, right? At least for MultiBistRunner, files should just be silently skipped.
In line with Octave's current runtests behavior, it keeps a list of all the .m or .cc files which had no tests, emits a "???? file had no tests" message in the verbose log output, and includes them in the list of "files with no tests" in the results. I think runtests's current goal is to nudge people closer to getting to 100% test coverage.
This should probably be adjusted for private/ and +internal functions; there probably should be no expectation that they have tests defined.
Fixed in d12a995.
After:
>> test2 testpkg/inst/foo.m
????? /Users/janke/tmp/testify-scratch/testpkg/inst/foo.m has no tests available
>> test2 testpkg/src/foo.cc
????? /Users/janke/tmp/testify-scratch/testpkg/src/foo.cc has no tests available
>> runtests2 testpkg
Summary:
GNU Octave Version: 4.4.1 (hg id: 1f46d371968c + patches)
Tests run on angharad.local (macOS) at 17-May-2019 20:21:17
Test execution time: 00:00:00
PASS 0
FAIL 0
>> runtests2 testpkg/inst
Summary:
GNU Octave Version: 4.4.1 (hg id: 1f46d371968c + patches)
Tests run on angharad.local (macOS) at 17-May-2019 20:21:19
Test execution time: 00:00:00
PASS 0
FAIL 0
>>
So, I think those runtests2 outputs should probably include a note about source files that had no tests. But we can handle that under a different issue if that should really be the behavior.
Can you confirm this fixes it for you?
👍 Works perfectly now
Great. Thanks for the report and confirmation.