On a fresh clone on Windows, the test suite fails 2 of 261 tests, both in fire/main_test.py and both Windows-specific. CI currently runs macos-latest + ubuntu-latest only, so neither can be caught there.
FAILED fire/main_test.py::MainModuleTest::testArgPassing - re.PatternError: bad escape \p at position 5
FAILED fire/main_test.py::MainModuleFileTest::testFileNameFire - PermissionError: [Errno 13] Permission denied: 'C:\\...\\tmponwjvj2u.py'
2 failed, 259 passed in 6.30s
Environment: Windows 11, Python 3.13.13, pip install -e . pytest hypothesis, clone at current master.
1. testArgPassing — unescaped path in a regex. assertOutputMatches treats its argument as a regex, and the test interpolates os.path.join('part1', 'part2', 'part3') unescaped. On Windows that's part1\part2\part3, and \p has been a hard error (re.PatternError) since Python 3.12. POSIX never sees it because / needs no escaping. The open PR #679 fixes exactly this with re.escape(); applying its change locally takes the suite to 1 failed / 260 passed, so it would be lovely to see it merged.
2. testFileNameFire — NamedTemporaryFile can't be reopened while open on Windows. MainModuleFileTest.setUp keeps the temp .py file open, and fire.__main__.import_from_file_path → exec_module then opens it a second time for import. That second open is the documented Windows limitation of NamedTemporaryFile ("the name cannot be used to open the file a second time while it is still open"), and it surfaces as PermissionError inside the import machinery. This is test-harness-only: python -m fire somefile.py itself works fine on Windows.
Fix I verified locally for (2) — with it (plus #679's change), the whole suite passes on this machine, 261 passed in 1.65s, and it stays green on POSIX semantics:
def setUp(self):
super().setUp()
- self.file = tempfile.NamedTemporaryFile(suffix='.py') # pylint: disable=consider-using-with
+ self.file = tempfile.NamedTemporaryFile(suffix='.py', delete=False) # pylint: disable=consider-using-with
self.file.write(b'class Foo:\n def double(self, n):\n return 2 * n\n')
- self.file.flush()
+ self.file.close()
+ self.addCleanup(os.unlink, self.file.name)
- self.file2 = tempfile.NamedTemporaryFile() # pylint: disable=consider-using-with
+ self.file2 = tempfile.NamedTemporaryFile(delete=False) # pylint: disable=consider-using-with
+ self.file2.close()
+ self.addCleanup(os.unlink, self.file2.name)
(delete_on_close=False would be tidier but is 3.12+, and setup.py still advertises 3.7 support.)
I'm reporting this as an issue rather than sending the patch because I haven't signed the Google CLA yet. Happy for anyone — including the author of #679 — to fold the diff above into a PR. Also happy to follow up with a windows-latest line for the CI matrix discussion if that's of interest, though I understand that's a bigger decision than these two fixes.
On a fresh clone on Windows, the test suite fails 2 of 261 tests, both in
fire/main_test.pyand both Windows-specific. CI currently runsmacos-latest+ubuntu-latestonly, so neither can be caught there.Environment: Windows 11, Python 3.13.13,
pip install -e . pytest hypothesis, clone at current master.1.
testArgPassing— unescaped path in a regex.assertOutputMatchestreats its argument as a regex, and the test interpolatesos.path.join('part1', 'part2', 'part3')unescaped. On Windows that'spart1\part2\part3, and\phas been a hard error (re.PatternError) since Python 3.12. POSIX never sees it because/needs no escaping. The open PR #679 fixes exactly this withre.escape(); applying its change locally takes the suite to 1 failed / 260 passed, so it would be lovely to see it merged.2.
testFileNameFire—NamedTemporaryFilecan't be reopened while open on Windows.MainModuleFileTest.setUpkeeps the temp.pyfile open, andfire.__main__.import_from_file_path→exec_modulethen opens it a second time for import. That second open is the documented Windows limitation ofNamedTemporaryFile("the name cannot be used to open the file a second time while it is still open"), and it surfaces asPermissionErrorinside the import machinery. This is test-harness-only:python -m fire somefile.pyitself works fine on Windows.Fix I verified locally for (2) — with it (plus #679's change), the whole suite passes on this machine,
261 passed in 1.65s, and it stays green on POSIX semantics:def setUp(self): super().setUp() - self.file = tempfile.NamedTemporaryFile(suffix='.py') # pylint: disable=consider-using-with + self.file = tempfile.NamedTemporaryFile(suffix='.py', delete=False) # pylint: disable=consider-using-with self.file.write(b'class Foo:\n def double(self, n):\n return 2 * n\n') - self.file.flush() + self.file.close() + self.addCleanup(os.unlink, self.file.name) - self.file2 = tempfile.NamedTemporaryFile() # pylint: disable=consider-using-with + self.file2 = tempfile.NamedTemporaryFile(delete=False) # pylint: disable=consider-using-with + self.file2.close() + self.addCleanup(os.unlink, self.file2.name)(
delete_on_close=Falsewould be tidier but is 3.12+, and setup.py still advertises 3.7 support.)I'm reporting this as an issue rather than sending the patch because I haven't signed the Google CLA yet. Happy for anyone — including the author of #679 — to fold the diff above into a PR. Also happy to follow up with a
windows-latestline for the CI matrix discussion if that's of interest, though I understand that's a bigger decision than these two fixes.