files_io doctest-> pytest #908
Conversation
| @@ -528,29 +502,9 @@ class OpenRead(_OpenAction): | |||
|
|
|||
| >> OpenRead["ExampleData/EinsteinSzilLetter.txt", CharacterEncoding->"UTF8"] | |||
| = InputStream[...] | |||
| #> Close[%]; | |||
| >> Close[%]; | |||
|
|
|||
| S> Close[OpenRead["https://raw.githubusercontent.com/Mathics3/mathics-core/master/README.rst"]]; | |||
There was a problem hiding this comment.
I keep these sandbox tests for another round, because I need to figure out how to mark them in the pytests. Any idea?
There was a problem hiding this comment.
I don't have a thought right now about sandboxed tests.
As for #> Close[%]; vs. >> Close[%];, if it is instructive that is okay. For example:
>> OpenRead["ExampleData/EinsteinSzilLetter.txt", CharacterEncoding->"UTF8"]
= InputStream[...]
Streams should be closed when done with them:
>> Close[%];But if it is not, but just done to make testing more robust and for bookkeeping, then keep the #> so we don't pollute the docs.
There was a problem hiding this comment.
Good: I leave then S>for another round. Also, I have reverted #> Close ->>> Close when it is not instructive.
There was a problem hiding this comment.
In Sphinx, the cleanup is implemented in a special code block directive testcleanup:
https://www.sphinx-doc.org/en/master/usage/extensions/doctest.html
|
LGTM - thanks. Sorry I didn't get to this sooner. |
| None, | ||
| ), | ||
| ( | ||
| 'OpenRead["MathicsNonExampleFile"]', |
There was a problem hiding this comment.
@mmatera There are a problems with this test.
It assumes this file does not exist. Another test in this parameterize loop creates a file with exactly this name.
Instead, the test should ensure that a file does not exist is used. One way to do this is to create a new named temporary directory which is empty and then look for some name there. https://docs.python.org/3/library/tempfile.html#tempfile.TemporaryDirectory can help here.
So probably this should be removed from this parameterize loop and be put in a separate test that has the additional code
| None, | ||
| ), | ||
| ( | ||
| 'fd=OpenRead["ExampleData/EinsteinSzilLetter.txt", BinaryFormat -> True, CharacterEncoding->"UTF8"]//Head', |
There was a problem hiding this comment.
@mmatera The sequence of tests from here to line 172 should be put in a separate test. Subsequent tests depend on the actions of earlier tests. If an early test fails, there is no cleanup. I am seeing for example "MathicsNonExampleFIle" appearing in the file system on failure and equally worse, in the mathics-core directory which should not be touched by testing.
In that new separate test we should be doing cleanup when something goes wrong or a test fails. Context managers can help, or possibly the use of a temporary new temporary file or directory.
For tests that are writing files, I do not think we are ensuring that we have access to a writable directory.
| @@ -82,6 +81,185 @@ def test_export(): | |||
| assert data.endswith("</svg>") | |||
|
|
|||
|
|
|||
| """ | |||
There was a problem hiding this comment.
Double quotes and comments? Comments with a TODO is probably good enough.
Here I started to take some nontrivial decisions: