doctest error with compare_versions aborts test run

apjanke commented 5 years ago

Now that we've removed many try/catches (#8), internal errors in the testing packages will abort the test run.

This is happening when testing the interval package, apparently in doctest due to a bad call to compare_versions.

To reproduce:

testify.install_and_test_forge_pkgs({"interval"}, {"doctest",true})

Example run: octave-testify-ForgePkgTester-2019-03-18_12-49-47.zip

/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:458: misplaced }
/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:469: @include: could not find image/cameleon.m.texinfo
/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:473: unknown command `myimage'
/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:473: misplaced {
/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:473: misplaced }
/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:484: unknown command `myimage'
/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:484: misplaced {
/var/folders/_4/9mx5ryp52bb_z6drbcbrhwl40000gn/T/octave-help-SXzVuC:484: misplaced }
      examples.texinfo ................................. error: Invalid call to compare_versions.  Correct usage is:

 -- compare_versions (V1, V2, OPERATOR)

File droppings were left by interval:
  advanced-topics.texinfo
  examples.texinfo
  getting-started.texinfo
  ieee-1788.texinfo
  introduction-to-interval-arithmetic.texinfo
  preface.texinfo

Results in spuriously reporting that all tests for the last package passed.

apjanke commented 5 years ago

Here's the stack trace:

debug> warning: implicit conversion from matrix to sq_string
warning: called from
    print_usage at line 91 column 5
    compare_versions at line 77 column 5
    doctest_run_tests at line 49 column 3
    doctest_run_docstring at line 96 column 9
    doctest_collect at line 179 column 11
    doctest_collect at line 117 column 13
    doctest at line 289 column 11
    __test_pkgs__ at line 156 column 38
    install_and_test_forge_pkg_unsafe at line 284 column 15
    install_and_test_forge_pkg at line 218 column 14
    install_and_test_forge_pkgs at line 167 column 13
    install_and_test_forge_pkgs at line 48 column 3
dbstack
stopped in:

  -->                       print_usage at line 91 [/Applications/Octave-5.1.0.app/Contents/Resources/usr/Cellar/octave-octave-app@5.1.0/5.1.0/share/octave/5.1.0/m/help/print_usage.m]
                       compare_versions at line 77 [/Applications/Octave-5.1.0.app/Contents/Resources/usr/Cellar/octave-octave-app@5.1.0/5.1.0/share/octave/5.1.0/m/miscellaneous/compare_versions.m]
                      doctest_run_tests at line 49 [/Users/janke/Library/Application Support/Octave.app/5.1.0/pkg/doctest-0.6.1/private/doctest_run_tests.m]
                  doctest_run_docstring at line 96 [/Users/janke/Library/Application Support/Octave.app/5.1.0/pkg/doctest-0.6.1/private/doctest_run_docstring.m]
                        doctest_collect at line 179 [/Users/janke/Library/Application Support/Octave.app/5.1.0/pkg/doctest-0.6.1/private/doctest_collect.m]
                        doctest_collect at line 117 [/Users/janke/Library/Application Support/Octave.app/5.1.0/pkg/doctest-0.6.1/private/doctest_collect.m]
                        doctest_collect at line 117 [/Users/janke/Library/Application Support/Octave.app/5.1.0/pkg/doctest-0.6.1/private/doctest_collect.m]
                        doctest_collect at line 117 [/Users/janke/Library/Application Support/Octave.app/5.1.0/pkg/doctest-0.6.1/private/doctest_collect.m]
                                doctest at line 289 [/Users/janke/Library/Application Support/Octave.app/5.1.0/pkg/doctest-0.6.1/doctest.m]
                          __test_pkgs__ at line 156 [/Users/janke/local/repos/octave-testify/inst/__test_pkgs__.m]
      install_and_test_forge_pkg_unsafe at line 284 [/Users/janke/local/repos/octave-testify/inst/+testify/+internal/ForgePkgTester.m]
             install_and_test_forge_pkg at line 218 [/Users/janke/local/repos/octave-testify/inst/+testify/+internal/ForgePkgTester.m]
            install_and_test_forge_pkgs at line 167 [/Users/janke/local/repos/octave-testify/inst/+testify/+internal/ForgePkgTester.m]
            install_and_test_forge_pkgs at line 48 [/Users/janke/local/repos/octave-testify/inst/+testify/install_and_test_forge_pkgs.m]
debug>

apjanke commented 5 years ago

That part in doctest_run_tests line 49 is actually an eval, not a call to compare_versions.

  % determine whether test should be skipped
  % (careful about Octave bug #46397 to not change the current value of “ans”)
  eval (strcat ('DOCTEST__current_test.skip = ', ...
                 doctest_join_conditions(DOCTEST__current_test.skip), ...
                ';'));

And the uses of compare_versions within doctest look okay:

$ grep -r compare_versions *                                                                                                           master
inst/private/doctest_compare.m:    if is_octave && compare_versions (OCTAVE_VERSION, '4.1', '<')
test/test_angle_brackets.m:% >> oct38 = DOCTEST_OCTAVE && compare_versions(OCTAVE_VERSION, '4.0.0', '<');
[~/local/repos/octave-doctest]
$

Maybe there's a bad compare_versions call in the interval package's embedded doctest strings.

apjanke commented 5 years ago

Ah, yep. In interval:

$ grep -r compare_versions *
doc/chapter/examples.texinfo:@c doctest: +SKIP_IF(compare_versions (ver ("symbolic").Version, "2.5.0", "<"))
doc/chapter/examples.texinfo:@c doctest: +SKIP_IF(compare_versions (ver ("symbolic").Version, "2.5.0", "<"))

If the symbolic package is not installed, then ver ("symbolic") returns an empty struct, and dot-referencing into causes the comma-separated list of function parameters to degenerate.

I think I'll work around this now by just adding interval to the known-bad list, and ask the doctest folks how they want to handle this.

apjanke commented 5 years ago

Worked around in https://github.com/apjanke/octave-testify/commit/db9da33f6c1f69267f76c04f4a19923934ebf272.

TODO: Report this bug in interval upstream. But we can get to that later.

cbm755 commented 5 years ago

Please do file upstream, or let me know if you want me to. Its also a bug in Interval (but of course, Doctest should be robust to such things.)

I guess we need a try-catch block around the evaluation of directives if we don't already.

apjanke commented 5 years ago

Filed upstream: https://github.com/catch22/octave-doctest/issues/205.

I'll get around to filing it upstream with interval (and symbolic) too, once the other Testify issues are fixed. Identifying bugs for filing is the main point of this package, after all.

cbm755 commented 5 years ago

I filed for Interval here: https://savannah.gnu.org/bugs/index.php?55969

apjanke commented 5 years ago

Thanks.

Not sure if there's a need to file for Symbolic, I think. The error that it raises is because SymPy is not installed, and SymPy is a requirement for Symbolic. I don't think the doctest tests would be expected to succeed in that case.

cbm755 commented 5 years ago

Yeah almost all tests should fail in that case

apjanke / octave-testify

doctest error with compare_versions aborts test run #44