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

regression: runtests2 error when called on a directory #81

Closed mtmiller closed 5 years ago

mtmiller commented 5 years ago

In testify 0.3.0, the following worked as expected when inst is a directory containing m-files

>> runtests2 inst
Processing files in /home/mike/src/octave/goodies/inst:

  basename.m .................................................. PASS     26/26  
  dirname.m ................................................... PASS     26/26  
  goodies.m ................................................... PASS      1/1   
  grep.m ...................................................... PASS     10/10  
  showlogo.m .................................................. PASS      1/1   

In testify 0.3.2, an error is raised

>> runtests2 inst
error: MultiBistRunner.add_file: file is a directory: /home/mike/src/octave/goodies/inst
error: called from
    add_file at line 126 column 9
    add_target_auto at line 81 column 9
    runtests2 at line 84 column 9
mtmiller commented 5 years ago

Ok, this started with 91d8fe76e71a, which moves the new implementations in place, and apparently I now need to use runtests2 -dir inst. Is that right and expected? If so, please go ahead and close this.

apjanke commented 5 years ago

Is that right and expected?

Nope, that's a bug. I'll fix.

apjanke commented 5 years ago

A bit more detail: the different functions for adding individual files, directories, and so on, are for Testify's internal use, to make execution paths more clear. The -dir option to runtests2 is mostly for debugging so I can invoke it directly from the command line and see how it behaves. When you specify something without an explicit -dir or other option, runtests2 is supposed to automatically detect what it is, and "do the right thing". The fact that it's erroring here means that add_target_auto is calling the wrong function internally, which is a bug.

apjanke commented 5 years ago

The problem was that the add_target_auto was checking the target to see whether it is a file or directory, and doing this:

      if ! isempty (canon_path) && exist (canon_path, "file")
        this.add_file (canon_path, tag);
        return
      elseif ! isempty (canon_path) &&  exist (canon_path, "dir")
        this.add_directory (canon_path, tag);
        return

forgetting that directories also count as "file"s.

I've fixed it by reversing the test order and doing the more-specific "dir" test first. (You could also fix it by checking exist()'s return value code, or writing a more-specific exist_plain_file function.)

Fix is checked in. This is an annoying enough bug that I'll roll a new point release, if you can confirm the fix works for you.

mtmiller commented 5 years ago

Yes, fix works perfectly

apjanke commented 5 years ago

Rolled release 0.3.3 with this fix.