search

apjanke / octave-testify

New BIST (Built-In Self Test) functions for GNU Octave
GNU General Public License v3.0
4 stars 2 forks source link

Unexpected errors on files without tests #83

Closed mtmiller closed 5 years ago

mtmiller commented 5 years ago

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
apjanke commented 5 years ago

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.

mtmiller commented 5 years ago

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 .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.

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.

apjanke commented 5 years ago

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.

apjanke commented 5 years ago

Fixed in https://github.com/apjanke/octave-testify/commit/d12a9951d6ed40824f222d09124759e552389b45.

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?

mtmiller commented 5 years ago

:+1: Works perfectly now

apjanke commented 5 years ago

Great. Thanks for the report and confirmation.