Handle the "ans" variable in the context
Abdallah-Elshamy opened this issue ยท 6 comments
The ans
variable is currently supported if it is printed in the output. We want to support if it is assigned but the new value is not printed
Sorry for the late reply and thank you for your changes @Abdallah-Elshamy ๐
While running the test suite, test JupyterNotebook
, I receive some error messages. Looking closer at the code, I often see this constructs:
pkg-jupyter-notebook/src/JupyterNotebook.m
Lines 721 to 722 in aa70c4a
This seems a little fragile to test for the exact shape of an error message ๐ In every Octave release a whitespace for example might be introduced and all our BIST are failing ๐
Therefore, I would prefer to not test for such error strings and remove those tests from the BISTs. Rather checking if the working output is correct, seems more "safe" to me. Although here whitespace might change over time, let alone format compact/losse/long/short
might ruin such checks...
Even though this might be less satisfying, maybe it is safer if we reduce the whole BIST to plain functional tests, i.e., no errors occur while running your JupyterNotebook
class functions, rather than checking string outputs byte-wise.
If you want to release the package, you can start any time on Octave packages ๐ Asking for some testers of your freshly released package on Octave Discourse might also bring up some unforeseen bug reports.
I think, once we got the BISTs in shape (next week) we push your class and BIST to core Octave.
Therefore, I would prefer to not test for such error strings and remove those tests from the BISTs. Rather checking if the working output is correct, seems more "safe" to me. Although here whitespace might change over time, let alone
format compact/losse/long/short
might ruin such checks...
In the past couple of days, I was looking for a more reliable way to test for those changes as they test a core feature of the package but I couldn't find any, unfortunately ๐ . I also agree with you that they are very fragile and it is better to remove those tests.
Even though this might be less satisfying, maybe it is safer if we reduce the whole BIST to plain functional tests, i.e., no errors occur while running your
JupyterNotebook
class functions, rather than checking string outputs byte-wise.
The tests for embedding images should be perfectly fine as they are embedded by the package and they are not run by Octave. Markdown cells are also treated in the same way. I think it is better to leave them as they are and reduce only the tests for embedding textual output to test for the structure of the notebook (the correct keys exist in any struct
in the outputs
). What do you think :quri?
I pushed a commit modifying the tests. Please, check it and if you think that the tests require more adjustments, let me know.
If you want to release the package, you can start any time on Octave packages wink Asking for some testers of your freshly released package on Octave Discourse might also bring up some unforeseen bug reports.
If you want to release the package, you can start any time on Octave packages wink Asking for some testers of your freshly released package on Octave Discourse might also bring up some unforeseen bug reports.
I think, once we got the BISTs in shape (next week) we push your class and BIST to core Octave.
That sounds great ๐ช. Once we fix the BISTs issues we can do that.
@siko1056 , Any updates?
Sorry for letting you wait. I wrote an email with further suggestions.
As I have hijacked this report with the testing issue, let us return to the original issue with the ans variable, which is actually solved by you two weeks ago. Great work ๐ ๐