Tests fail on windows because of newline about beet HOT 6 CLOSED

Good catch! File handles are lazy and don't load or parse their content unless absolutely necessary so right now when comparing files for equality I'm using their serialized representation. One change I actually made a few days ago was implementing comparisons using the deserialized value for json files as I was running into a different field ordering in CI. But it's still not ideal so I just made the __eq__ implementation a bit smarter. There's a new fast path when both files have the same source_path that avoids unnecessary loads, and the comparison falls back to the deserialized value if the serialized representation wasn't the same.

This should take care of the inconsistency with line endings on windows. Other than that there's not really any os-specific behavior, so I'm probably not going to start running tests on windows. I really like to keep my CI as fast as possible so unless I actually have platform-specific code in a project I try to keep it linux-only.

Tell me if you encounter any other issue with this.

from beet.

In that case I'm going to play the part of the botnet matrix. Call me Neo :D

from beet.

Whoa thanks! Sorry I was busy. I didn't anticipate this but you're right, resolving the config file produces different paths on windows. I'm fine with the fix_paths function but instead of doing the adjustments before comparing the snapshot, I ended up handling this in a custom snapshot format that converts the paths to the current platform when the snapshot gets loaded d4d3f91

I also changed the cache matching test to use test-foo instead of test:foo. While I was at it I set up the project on windows on my end, and it looks like everything's working fine now, so I'm gonna close the issue but again let me know if anything else comes up.

from beet.

The saga continues.
The newline fix worked, no tests fail because of it anymore (from 36 failing down to 12).
But the second Windows/Linux differences are the paths, \ vs /:

Excerpt:

snapshot = snapshot, directory = 'whitelist_nested'

    @pytest.mark.parametrize("directory", os.listdir("tests/config_examples"))  # type: ignore
    def test_config_resolution(snapshot: Any, directory: str):
        project_config = load_config(f"tests/config_examples/{directory}/beet.json")
>       assert snapshot("json") == project_config.dict()

⠇

                 'description': '',
E         -                'directory': 'tests\\config_examples\\whitelist_nested',
E         ?                                   ^^               ^^
E         +                'directory': 'tests/config_examples/whitelist_nested',

⠇

tests\test_config.py:12: AssertionError
12 failed, 83 passed, 8 skipped in 4.86s

Also a quirk I noticed with OSX in my own repos: Apple seems to have a different file ordering, had an issue where I assumed the same order of reading files on every OS.

from beet.

Issue seems to be the ProjectConfig class. Its directory attribute seems to be stored as a string, therefore the platform dependent paths. Idealy this could be fixed by changing the attribute to a pathlib.Path. However, since this is only a problem that occurs during tests on Windows, that seems overkill.

I quickly tested with a hacky fix that just converts all path strings to posix path strings:

def fix_paths(cfg: ProjectConfig):
    if cfg.directory:
        cfg.directory = Path(cfg.directory).as_posix()
    if cfg.output:
        cfg.output = Path(cfg.output).as_posix()

    cfg.data_pack.load = [Path(p).as_posix() for p in cfg.data_pack.load]
    cfg.resource_pack.load = [Path(p).as_posix() for p in cfg.resource_pack.load]

    if cfg.templates:
        cfg.templates = [Path(p).as_posix() for p in cfg.templates]

    for entry in cfg.pipeline:
        if isinstance(entry, ProjectConfig):
            fix_paths(entry)

    return cfg


@pytest.mark.parametrize("directory", os.listdir("tests/config_examples"))  # type: ignore
def test_config_resolution(snapshot: Any, directory: str):
    project_config = load_config(f"tests/config_examples/{directory}/beet.json")
    fix_paths(project_config)
    assert snapshot("json") == project_config.dict()

That seems to fix all the problems with Paths.
But Windows is a vile beast that doesn't go down with even 2 hits:

tmp_path = WindowsPath('C:/Users/Michael/AppData/Local/Temp/pytest-of-Michael/pytest-55/test_match0')

def test_match(tmp_path: Path):        
        with MultiCache(tmp_path) as cache:
>           cache["test:foo"]

tests\test_cache.py:155:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
beet\core\container.py:132: in __getitem__
    value = self.missing(key)
beet\core\cache.py:179: in missing
    cache = Cache(self.path / key)
beet\core\cache.py:35: in __init__
    if self.index_path.is_file()
C:\Python39\lib\pathlib.py:1439: in is_file
    return S_ISREG(self.stat().st_mode)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
self = WindowsPath('C:/Users/Michael/AppData/Local/Temp/pytest-of-Michael/pytest-56/test_match0/test:foo/index.json')

E       OSError: [WinError 123] Die Syntax für den Dateinamen, Verzeichnisnamen oder die Datenträgerbezeichnung ist falsch: 'C:\\Users\\Michael\\AppData\\Local\\Temp\\pytest-of-Michael\\pytest-56\\test_match0\\test:foo\\index.json

Now Windows personally complains (thats why the error is in German, sry). Seems the cache checks 'C:/Users/Michael/AppData/Local/Temp/pytest-of-Michael/pytest-56/test_match0/test:foo/index.json' with path.is_file(). But : is not allowd in file names on Windows, thus the cache file test:foo fails

from beet.

Great stuff, tests work like a charm now. Thanks for all the effort :D

from beet.