Fixes for Tests under Windows. #407
Conversation
In differ.py, when 'differ' walks a dir tree to make a list of files in a ufo file, Windows and Mac list the files in a different order. The lists are compared, and the diff fails before the files are even examined. in tx_test.py, when pdf files are compared, an additional line in the pdf file needs to be ignored. This didn't cause a problem before, but as of June 1st, the date string changed enough to make this extra line change.
Replaced tmpfileNameTempFile() with tmpfile.mkstemp(). The former not only creates a file, but also opens it. This appears to conflict with writing to the same file path from other processes. In any case, this change fixes a lot of failures that appears on some, but not all, Windows systems.
Tests/differ.py
Outdated
| @@ -163,6 +163,7 @@ def _get_all_file_paths_in_dir_tree(start_path): | |||
|
|
|||
| # Make the paths relative | |||
| all_paths = [path.replace(start_path, '') for path in all_paths] | |||
| all_paths.sort() | |||
There was a problem hiding this comment.
I don't think this is necessary because the paths are not going to be compared between Win and Mac
There was a problem hiding this comment.
Sorry, I made a mistake in the comment - the difference is between the reference directory tree and the new temporary director tree when the test is run under Windows, not Mac and Windows.
There was a problem hiding this comment.
OK, then use sorted() instead.
Use absolute path for tx.exe
Referencing the absolute path to tx.exe fixes a failure when running the test under Appveyor.. Saving the output from a shell call to tx.exe to a file name rather than a tmpfile.TempFile file object fixes a failure under Windows OSX 10 when running pytest manually.
…stead of NameTempFile().
Tests/differ.py
Outdated
| @@ -196,6 +197,9 @@ def _paths_are_same_kind(path1, path2): | |||
|
|
|||
|
|
|||
| def _validate_path(path_str): | |||
| if path_str is None: | |||
| raise argparse.ArgumentTypeError( | |||
| "{} is not a valid path.".format(path_str)) | |||
There was a problem hiding this comment.
I don't think this validation is needed because this method will only be called whent the command line argument is provided; if it's not provided things will fail earlier
There was a problem hiding this comment.
@miguelsousa I put the code in because there is a path where this not caught. In runnner.run_test(), if either of the subprocess calls encounters an exception, it is caught and the function returns None as the output path. Most of the test functions receive this as 'actual_path', and then call differ(). In differ.main(), the get_options() call converts the "None" to the current directory. The result is a mis-leading error message that the path types differ, as the expected_path is a file, and the actual_path is a directory.
There was a problem hiding this comment.
OK, in that case please add a test case for this situation.
|
@readroberts don't forget to bump the versions of |
Note that I did not handle the 'subprocess.CalledProcessError' in runner.py::main(). I notice that all the exceptions that can be raised by the argparse module are not handled, and there is no exception handling for any other exception. I think it is simpler to let the 'subprocess.CalledProcessError' pass through and be caught and reported by pytest.
…l when there is a CalledProcessError.
…grams a a file of their intended output file type as input. The programs are not designed to fail in any reliable way in this condition. The current tests cause detype1 to fail with the same error on Mac and Windows, but do not cause type1 to fail.
…it will not cause derivation of the input directory to fail.
…process call does not fail, as well when it fails for a reason other than CalledProcessError.
|
@readroberts all the commits to |
|
@msousa @cchapman I still can't reproduce this on my local Windows system. I see the the errors are only with the '-dump' option of tx. This is what is causing the failures in the ttxn command and the fontplot commands as well. I might have suspected a problem with writing to stdout, but in most of the errors, an output file is supplied as an explicit option to tx. When I get back to work Mon afternoon, I will focus on trying to generate a reproducible crash case. |
* In differ.py, when it walks a dir tree to make a list of files in a ufo file, the list needs to be sorted to avoid the difference between the reference directory tree and the new temporary director tree when the test is run under Windows. The lists were compared, and the diff was failing before the files were even examined. * In tx_test.py, when pdf files are compared, an additional line in the pdf file needs to be ignored. This didn't cause a problem before, but as of June 1st, the date string changed enough to make this extra line change. * Replaced tmpfileNameTempFile() with tmpfile.mkstemp(). The former not only creates a file, but also opens it. This appears to conflict with writing to the same file path from other processes. In any case, this change fixes a lot of failures that appears on some, but not all, Windows systems. * Use absolute path for tx.exe * Fixes for Test under Windows with ttxn. Referencing the absolute path to tx.exe fixes a failure when running the test under Appveyor. Saving the output from a shell call to tx.exe to a file name rather than a tmpfile.TempFile file object fixes a failure under Windows OSX 10 when running pytest manually. * Shift to consistent use of tempfile.mkstemp() instead of NameTempFile(). * Add test for when command fails. Note that I did not handle the 'subprocess.CalledProcessError' in runner.py::main(). I notice that all the exceptions that can be raised by the argparse module are not handled, and there is no exception handling for any other exception. I think it is simpler to let the 'subprocess.CalledProcessError' pass through and be caught and reported by pytest. * Add some debug code to show path to tool when there is a CalledProcessError. * Remove type1 and detype1 tests that run the programs on a file of their intended output file type as input. The programs are not designed to fail in any reliable way in this condition. The current tests cause detype1 to fail with the same error on Mac and Windows, but do not cause type1 to fail.
In differ.py, when 'differ' walks a dir tree to make a list of files in a ufo file, Windows and Mac list the files in a different order. The lists are compared, and the diff fails before the files are even examined.
in tx_test.py, when pdf files are compared, an additional line in the pdf file needs to be ignored. This didn't cause a problem before, but as of June 1st, the date string changed enough to make this extra line change.