fix CI for Windows platforms

[dlqqq]

Testing CI step was added in #4, but support for Windows and Ubuntu pypy platforms had to disabled due to failure. These tests should be fixed and re-enabled to ensure platform compatibility.

[kevin-bates]

I spent some time on these today.

The pypy issue is, so far, completely related to sqlite. There are a couple different forms of issues. Setting the isolation_level to None to force manual commits helped a bit during DDL/DML transactions, but there appear to be issues related to connection management (second class instance can't get initialized).

The Windows issues are two-fold.

• For LocalFileIdManager it was more of a test issue because the case of the original path is normalized (which on Windows converts to lower-case) so the assertions of == fail. I have a helper method that looks at the fid-manager and normalizes the path for the comparison.
• The second issue is with the ArbitraryFileIdManager's use of os.path.join() or any os.path methods that deal with separators. I think it's important that the AFIM be completely filesystem agnostic. To compose "absolute" paths, we should use the "/" unconditionally (i.e., not rely on os.path). For users that need to mix Windows and Linux clients, AFIM may want to have an algorithm for how to normalize source_root - but that can wait.

[dlqqq]

I spent some time on these today.

Thank you so much! I spent all of Friday banging my head against this, so it's encouraging to know I had good company 😁. Just curious, did you already have a Windows development environment set up?

The pypy issue is, so far, completely related to sqlite. There are a couple different forms of issues. Setting the isolation_level to None to force manual commits helped a bit during DDL/DML transactions, but there appear to be issues related to connection management (second class instance can't get initialized).

I was able to reproduce this on Friday in a separate conda environment:

conda create -n pypy
conda activate pypy
conda install -c conda-forge pypy
pip install -e ".[test]"
pypy3 -m pytest

I tried some of the workarounds suggested here to no success. Specifically, changing the journal mode and calling fetchall() on every cursor.

It looks like there are some quirks specific to pypy's implementation of sqlite3, so providing support for pypy may be more difficult than I had anticipated. I think this is lower priority than Windows compatibility, so I think it's best to track this in a separate issue and not let this block our efforts: #44

For LocalFileIdManager it was more of a test issue because the case of the original path is normalized (which on Windows converts to lower-case) so the assertions of == fail. I have a helper method that looks at the fid-manager and normalizes the path for the comparison.

Thanks for the hint! I'll take a look into this.

The second issue is with the ArbitraryFileIdManager's use of os.path.join() or any os.path methods that deal with separators. I think it's important that the AFIM be completely filesystem agnostic.

I 100% agree. I noticed this on Friday as well and had left it as a comment on #35.

[dlqqq]

@kevin-bates If it's not too big of an ask, do you think you could tackle this? I don't have a Windows development environment set up right now.

[kevin-bates]

Just curious, did you already have a Windows development environment set up?

No. I enabled CI for all pushed branches (which I find more helpful in that it gives users a chance to see how the CI goes before they create a pull request without having to first push to their main branch).

@kevin-bates If it's not too big of an ask, do you think you could tackle this? I don't have a Windows development environment set up right now.

I suspect I can tackle this w/o having to create a Windows VM, although I think ArbitraryFileIdManager will have to take some liberties to normalize "root_dir" if there are plans to support this test. The liberties I'm referring to are to replace \ with / and remove adjacent slashes (i.e., // -> /) and avoid the use of os.path (including those used in the ABC) per my comment above.