Update __init__.py to fix #1982#1983
Conversation
AndreMiras
left a comment
There was a problem hiding this comment.
Looking good, thanks for investigating and fixing.
Having tests would be great too, do you want to give a stab at it?
|
Yes, I think I can try to add this too. I see your tests use the standard unit test library from python; and temporary files. If I understand your code and tests organization correctly, you'd suggest to add a test of Buildozer._copy_application_sources() to TestBuildozer class in buildozer/tests/test_buildozer.py? |
Yes that seems right to me, thanks for giving it a try |
|
OK, I need a few days and give you a proposal. |
|
Finally, as your tests didn't seem to cover Buidlozer._copy_application_sources at all, I have included tests to cover more of its code (not only the hidden parent case), though I am not 100% sure it covers all cases. In order to make changes in a cleaner way, I have gathered the tests of Buidlozer._copy_application_sources in a new dedicated class TestCopyApplicationSources(unittest.TestCase) with its own setUp, tearDown and helpers. So I think it's clean, clear and does not interfere with existing tests. It looks good, on my local linux I get this (I only check python 3.11 here): So I push this and let you check it. |
tests/test_buildozer.py
Outdated
| # def create_dir(self, relpath): | ||
| # """Helper to create a directory in source_dir.""" | ||
| # dirpath = os.path.join(self.source_dir, relpath) | ||
| # os.makedirs(dirpath, exist_ok=True) | ||
| # return dirpath |
There was a problem hiding this comment.
Let's remove this commented out code completely
AndreMiras
left a comment
There was a problem hiding this comment.
Thanks for taking a stab at it.
I didn't expect integration tests to be honest, but that also works I guess.
Let's maybe just remove the unused commented out code and I think we're good for merging.
Thanks again 🙇
2263844
|
OK, I added a commit to remove the commented out code! |
AndreMiras
left a comment
There was a problem hiding this comment.
Thanks for the follow up.
Merging 🚀
Problem is described here: #1982