Test suite fails on Windows: unescaped path regex in testArgPassing, NamedTemporaryFile reopen in testFileNameFire
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 2/5
- Tiempo estimado
- 1-3 horas
- Aptitud para principiantes
- 85/100
- Tipo de issue
- Error
- Claridad
- Bien especificado
- Estado de actividad
- Tranquilo
- Stack tecnológico
- python
- Área
- testing-qa
Línea de trabajo
Comienza en fire/main_test.py, en MainModuleTest.testArgPassing y MainModuleFileTest.setUp/testFileNameFire, y luego ejecuta la suite de pruebas de Windows con pytest. Comprueba el contexto existente de PR #679 para el fallo de regex y verifica que los archivos temporales se puedan volver a abrir durante la importación. La tarea está terminada cuando las 261 pruebas pasen en Windows y POSIX.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.
- Lenguaje dominante
- Python
- Estrellas
- 28.2k
- Forks
- 1.5k
- Métricas de merge de PR
- Sin PR fusionados en 30 d
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de google/python-fire
-
Release 0.7.2? Abierto
Dificultad 3/5 1-2 días Aptitud para principiantes 38/100
google/python-fire#698 ·
-
Dificultad 3/5 1-2 días Aptitud para principiantes 58/100
google/python-fire#672 · 5 comentarios ·
-
Dificultad 4/5 3-5 días Aptitud para principiantes 45/100
google/python-fire#665 · 2 comentarios ·
-
Dificultad 4/5 3-5 días Aptitud para principiantes 55/100
google/python-fire#659 · 1 comentario ·
-
Releasing 3.14 Support Abierto
Dificultad 3/5 1-2 días Aptitud para principiantes 45/100
google/python-fire#643 · 4 comentarios · 5 reacciones ·
Todos los issues de google/python-fire
Issues similares
-
documentation help wanted
Dificultad 2/5 1-3 horas Aptitud para principiantes 90/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 90/100
simonw/sqlite-utils#872 ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100