gh-79459 tempfile: Raise a ValueError if the prefix or suffix contains a directory component.#150477
gh-79459 tempfile: Raise a ValueError if the prefix or suffix contains a directory component.#150477mbyrnepr2 wants to merge 2 commits into
ValueError if the prefix or suffix contains a directory component.#150477Conversation
Raise a ``ValueError`` if the ``prefix`` or ``suffix`` contains a directory component. pythongh-79459
ValueError if the prefix or suffix contains a directory component.ValueError if the prefix or suffix contains a directory component.
|
|
||
|
|
||
| class TestPrefixAndSuffix(BaseTestCase): | ||
| def test_value_error_if_prefix_or_suffix_contains_directory(self): |
There was a problem hiding this comment.
| def test_value_error_if_prefix_or_suffix_contains_directory(self): | |
| def test_prefix_suffix_error(self): |
| ((os.sep), None), | ||
| (os.fsencode(os.sep), tempfile.gettempdirb()), | ||
| ) | ||
| else: |
There was a problem hiding this comment.
I don't think that it should be exclusive. I would suggest to always test with os.sep. And if os.altsep is defined (not None), test also os.altsep.
Instead of just passing sep/altsep, I would prefer 'dir{sep}name' relative path and '{sep}abs_name' absolute path.
| with self.assertRaisesRegex(ValueError, MESSAGE): | ||
| os.rmdir(tempfile.mkdtemp(dir=directory, prefix=value)) | ||
| with self.assertRaisesRegex(ValueError, MESSAGE): | ||
| tempfile.NamedTemporaryFile(dir=directory, prefix=value, delete=True) |
There was a problem hiding this comment.
I don't think that testing delete=True is relevant here.
You should test suffix as well.
| if suffix is None: | ||
| suffix = output_type() | ||
| if _os.path.dirname(suffix): | ||
| raise ValueError("'prefix' or 'suffix' can't contain a directory component") |
There was a problem hiding this comment.
Why not being more specific and only mention suffix here? And only mention prefix in the error below.
There was a problem hiding this comment.
Fair. I tried too hard to write as little code as possible.
| ((os.altsep), None), | ||
| (os.fsencode(os.altsep), tempfile.gettempdirb()), | ||
| ) | ||
|
|
There was a problem hiding this comment.
You should test all functions taking prefix/suffix arguments:
- mkstemp()
- mkdtemp()
- NamedTemporaryFile()
- TemporaryFile()
Write one test for prefix and a second test for the suffix.
| for value, directory in data: | ||
| with self.subTest((value, directory)): | ||
| with self.assertRaisesRegex(ValueError, MESSAGE): | ||
| tempfile.mkstemp(dir=directory, prefix=value) |
There was a problem hiding this comment.
I don't think that it's useful to set the dir parameter.
Co-authored-by: Victor Stinner <vstinner@python.org>
tempfile: Raise a
ValueErrorif theprefixorsuffixcontains a directory component.gh-79459