delph-in / pydelphin

Python libraries for DELPH-IN
https://pydelphin.readthedocs.io/
MIT License
77 stars 27 forks source link

Newline/encoding issues causing tests to fail on Windows #379

Closed goodmami closed 8 months ago

goodmami commented 8 months ago

It looks like newline issues are causing problems with running tests on windows:

     def test_update(self, single_item_table):
        table = single_item_table
>       assert table[0] == (0, 'The dog barks.')
E       AssertionError: assert <Row object (...2284478645696> == (0, 'The dog barks.')
E         At index 1 diff: 'The dog barks.\r' != 'The dog barks.'
E         Full diff:
E         - (0, 'The dog barks.')
E         + <Row object (0, 'The dog barks.\r') at 2284478645696>

This seems to affect the tsdb and tdl tests.

goodmami commented 8 months ago

The reason for errors in the tdl_test.py file is that the testing file writes to disk without specifying an encoding. This isn't a bug in PyDelphin proper, just in the tests.

https://github.com/delph-in/pydelphin/blob/33415cf45c1de47a9d895565062ef4ca731a378e/tests/tdl_test.py#L34-L39

goodmami commented 8 months ago

For the TSDB tests, the issue was similar in that the code is fine but the tests are buggy for Windows. In this case it's the use of Pathlib.Path.write_text() to write the testsuite files:

https://github.com/delph-in/pydelphin/blob/33415cf45c1de47a9d895565062ef4ca731a378e/tests/conftest.py#L54-L59

The write_text() method does not allow one to specify the newline parameter prior to Python 3.10 and it defaults to the system sequence. The solution here is to make a function that does a regular open on the files with newline="\n".

I think it is true that TSDB database files invariably use \n as line endings and never \r\n, even on Windows systems, but I cannot find a definitive source for that at the moment.