search

python-babel / babel

The official repository for Babel, the Python Internationalization Library
http://babel.pocoo.org/
BSD 3-Clause "New" or "Revised" License
1.29k stars 433 forks source link

Fix for #990 (updated) #1035

Open mdklatt opened 8 months ago

mdklatt commented 8 months ago

This is an update of https://github.com/python-babel/babel/pull/1006 that incorporates upstream fixes to previously-broken tests. It should pass all CI checks now.

This PR:

mdklatt commented 7 months ago

Would it be possible to formulate the tests without adding the new pyfakefs dependency?

The Unix tests are the only ones that use pyfakefs, so I can probably replace that with os.chroot.

akx commented 7 months ago

The Unix tests are the only ones that use pyfakefs, so I can probably replace that with os.chroot.

That sounds a bit heavy-handed too... What if you just mock.patch some of the relevant calls?

mdklatt commented 7 months ago

The Unix tests are the only ones that use pyfakefs, so I can probably replace that with os.chroot.

That sounds a bit heavy-handed too... What if you just mock.patch some of the relevant calls?

It turns out that os.chroot will not work anyway because it requires root privileges. However, the _unix._get_localzone() function takes an optional parameter specifically intended for testing, according to its docstring:

The parameter _root makes the function look for files like /etc/localtime beneath the _root directory. This is primarily used by the tests. In normal usage you call the function without parameters.

The get_localzone() function is just a wrapper (for now...) around _get_localzone('/'). I have a working commit that calls _get_localzone() with a temporary test directory.

It looks like it would only be necessary to mock os.path.join() in localtime._unix if you still think that's better.