emborg
emborg copied to clipboard
KenKundert
fix tests on macOS
Hello! I noticed that some of the tests are failing on macOS. I implemented a small fix. Happy to take feedback on it.
Details below.
The tests assume that the config dir is ~/.config and the data dir is ~/.local/share. Neither is true on macOS, and this causes several tests to fail:
FAILED tests/test_emborg.py::test_emborg_without_configs[4] - AssertionError: Test ‘labor’ fails.
FAILED tests/test_emborg.py::test_emborg_with_configs[5] - AssertionError: Test ‘periphery’ fails.
FAILED tests/test_emborg.py::test_emborg_with_configs[25] - AssertionError: Test ‘build’ fails.
FAILED tests/test_emborg.py::test_emborg_overdue[1] - AssertionError: Test ‘predator’ fails.
FAILED tests/test_emborg.py::test_emborg_overdue[2] - AssertionError: Test ‘smelt’ fails.
Rather than changing the tests to dynamically use different test paths based on the OS[1], it's simpler to change emborg's behavior at test time to always use the Linux paths. That is what this change does.
Also adds tests/Users to .gitignore, which is the macOS equivalent of tests/home.
[1] This was in fact the first thing I tried. But as I got into it the added complexity didn't seem justified.
Pull Request Test Coverage Report for Build 10426428059
Details
- 0 of 0 changed or added relevant lines in 0 files are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage remained the same at 63.94%
| Totals | |
|---|---|
| Change from base Build 10407960020: | 0.0% |
| Covered Lines: | 1970 |
| Relevant Lines: | 2887 |
💛 - Coveralls
I'm not crazy about modifying the emborg executable to support a special testing mode.
Is it possible to resolve this issue simply by adding some symbol links in the tests directory so that the paths needed by the mac exist and point to the ones provided for linux?
I'm afraid I do not have a Mac and have been unable to get testing with a Mac working in github actions, so it is difficult for me to resolve this myself.
Makes sense that you're reluctant to add test-only code to the executable.
Adding symbolic links is a nice idea, but unfortunately doesn't solve the problem.
It is possible to solve the problem by modifying only the tests and not the executable. I've pushed a new version that does that. As you can see, it's a significantly bigger change, but might be better depending on your taste. I'm interested to see what you think.
Yes, it is a much larger change. And I am starting to rethink your original version. As I said, I am not crazy about modifying the executable to support a testing mode, but I have previously heard from a Mac user that they use the Linux config file conventions on the Mac because they find the Mac conventions awkward. So if we change the EMBORG_TEST environment variable to EMBORG_CONFIG_DIR and EMBORG_LOG_DIR it turns the whole thing into a feature that allows users to choose the directories. Give me a couple of days to think about this. I am distracted at the moment but I will try to resolve this within a week or two.
And thank you for providing both implementations. I am sorry for putting you through the trouble of creating the second version.
Actually, there may be another related solution. Emborg uses the appdirs module to find the config and data directories. According to the appdirs documentation, it appears to use XDG, which suggests that one can control the location of the config and data dir using XDG_CONFIG_HOME and XDG_DATA_HOME environment variables. Unfortunately, on MacOS appdirs ignores XDG and uses hardcoded paths unique to the Mac. Rather than using EMBORG_CONFIG_DIR and EMBORG_LOG_DIR emborg could honor the XDG variables if defined and ignore the ones provided by appdirs.
I am not sure that this is better than going with emborg specific environment variables, but it is an alternative I am considering.
Thinking more about this, presumably the tests will not work on Windows machines any more than they work on Macs. And using your first solution will presumably resolve the issues on Windows while your second solution will not. That is another reason to go with the enviornment variables approach.
Here is another alternative. emborg could use the directory suggested by appdirs if it exists, otherwise is defaults to the XDG or Linux conventions.
In digging in to this I found that emborg was already configured to use ~/.config/avendesora if it existed, even on a Mac. This was due to an earlier request from a Mac user that did not care for the default location of configuration directories on the Mac. So presumably the problem you were having was not due to the config directory. Perhaps there is something else that is the issue.
I have modified emborg to support the XDG_CONFIG_HOME and XDG_DATA_HOME environment variables. If these environment variables are set, emborg will use their values as the location of the shared config and data directories. I set these values before starting the tests so the tests should always use the pre-configured config directory in the tests directory.
Please give it a try and see if it resolves your issue. The version of github is the one that contains the changes.
Oh, and I just saw your previous comment:
In digging in to this I found that emborg was already configured to use ~/.config/avendesora if it existed, even on a Mac. This was due to an earlier request from a Mac user that did not care for the default location of configuration directories on the Mac. So presumably the problem you were having was not due to the config directory. Perhaps there is something else that is the issue.
I had noticed this too. But the test setup does not automatically create this directory at first, so that logic does not trigger. I considered creating that directory in the test setup, but the test labor expects it not to exist, and checks that emborg creates it.
The XDG solution works well. Just wanted to add that note to the record 🙂