Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: --update-data incompatible with xdist #15641

Conversation

ikonst
Copy link
Contributor

@ikonst ikonst commented Jul 12, 2023

Fixes a gotcha with --update-data.

When using --update-data with parallelized tests (xdist), the line offsets determined at test time may become wrong by update time, as multiple workers perform their updates to the same file.

This makes it so we exit with a usage error when --update-data is used with parallelism.

@ikonst
Copy link
Contributor Author

ikonst commented Jul 12, 2023

For completeness: Another option would be to rewrite --update-data so that:

  1. when it enqueues changes, enqueue "update section Y of testcase X to Z" rather than "update line range A..B to Z".
  2. when it applies changes, parse the INI structure (right now we simply load the file as list[str]) and rewrite while replacing specific section(s)
    2.1. while holding a lock over the file

This is obviously more complex.

Copy link
Contributor

@A5rocks A5rocks left a comment

Choose a reason for hiding this comment

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

Thank you for catching my gotcha! I (as someone who doesn't know our own pytest configuration) assumed numprocesses wouldn't be defined, but now I see that this definitely works.

@JelleZijlstra JelleZijlstra merged commit 38a7104 into python:master Jul 12, 2023
12 checks passed
@ikonst ikonst deleted the 07-11-tests_--update-data_incompatible_with_xdist branch July 12, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants