Skip to content

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jan 26, 2025

  • Use f-string.
  • Fix grammar: replace 'datas' with 'data' (and replace 'data' with 'item').
  • Remove unused variables: 'pid' and 'old_mask'.

* Use f-string.
* Fix grammar: replace 'datas' with 'data' (and replace 'data' with
  'item').
* Remove unused variables: 'pid' and 'old_mask'.
@vstinner
Copy link
Member Author

cc @picnixz @cmaloney

Copy link
Contributor

@cmaloney cmaloney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely more modern. I duplicated a lot of code between test_read and test_readinto in GH-129211 I'm planning to dedupe (cmaloney@ad161bc#diff-60657ac311c452c61e0a872c2d0c7b6e763196617d65cfb3a7f73d791c922271L126). Happy to do that separately or could be incorporated here.

Might be out of scope, but the:

proc = self.subprocess(*args, **kwargs)
with kill_on_error(proc)
    ...
    self.assertEqual(proc.wait(), 0)

across cases I think could be deduplicated with a context manager:

with self.checked_subprocess(code, *args, **kwargs):
    ...


# the payload below are smaller than PIPE_BUF, hence the writes will be
# atomic
datas = [b"hello", b"world", b"spam"]

This comment was marked as duplicate.

'rd = int(sys.argv[1])',
'sleep_time = %r' % self.sleep_time,
'data = b"x" * %s' % support.PIPE_MAX_SIZE,
f'sleep_time = {self.sleep_time!r}',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would it make sense to centralize the "make a string which sets sleep_time" (f'sleep_time = {self.sleep_time!r}') rather than the number of copies? (Possibly also data as well?)

Definitely nice having all the code for a test case in one place though, but feels fairly templated/repeated in structure at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that it's worth it to have a helper just to format f'sleep_time = {self.sleep_time!r}' line.

Co-Authored-by: Cody Maloney <[email protected]>
@vstinner
Copy link
Member Author

I duplicated a lot of code between test_read and test_readinto in #129211 I'm planning to dedupe (cmaloney@ad161bc#diff-60657ac311c452c61e0a872c2d0c7b6e763196617d65cfb3a7f73d791c922271L126).

I integrated your change.

@vstinner vstinner merged commit 8a5a18a into python:main Jan 27, 2025
37 checks passed
@vstinner vstinner deleted the test_eintr branch January 27, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants