Skip to content

Non-parallel fixture with parallel children runs in parallel with other fixtures #2143

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

Closed
CharliePoole opened this issue May 2, 2017 · 2 comments

Comments

@CharliePoole
Copy link
Member

CharliePoole commented May 2, 2017

I'm breaking out this issue from the much larger complex of issues represented by #1474 because (1) it will be easier to solve and (2) it will require creating part of the solution for the larger issue.

Quoting from the above issue...

Let's say you have a [NonParallelizable] test fixture. It's executing (by itself, of course) from the non-parallelizable queue. Now let's say that some of the test methods in that fixture are marked [Parallelizable]. As they are encountered, they will be added to the Parallelizable queue.

Eventually, the dispatcher will exhaust the non-parallelizable queue and switech execution to the parallelizable queue and those test cases will get run. However, there may be other fixtures and their test cases on that queue and they will be run even though the original non-parallelizable fixture is still active.

[In fact, the problem can arise when any type of non-parallel test has parallelizable children - for example, a non-parallelizable SetUpFixture with parallelizable TestFixtures under it. In this issue, however, let's deal with the simplest case of a non-parallelizable TestFixture containing parallelizable tests.]

What we would like to see happen here is this:

  1. The fixture runs it's one-time setup in isolation, with no other fixtures running.
  2. All the test cases run in isolation from other fixtures, even if they run in parallel.
  3. The fixture runs it's one-time teardown, still in isolation.

In reality, only (1) and (3) happen correctly. That's because our notion of parallelism deals with what tests are running and a fixture is only considered to be running when it's own code (not that of it's children) is executing.

In fact, we need to define a notion of a fixture being active even if not running. It should be considered as active from the start of its one-time setup to the end of its one-time teardown. Our definition of parallelism has to switch from which tests are running in parallel to which tests are active in parallel. This is a big change, which is why I'm trying to start with a very limited test case.

As a first cut at the problem, I'm thinking of modifying how the parallel dispatcher is used.

Current Approach:

  • The top level (composite) workitem, representing the assembly, is used to initialize the dispatcher.
  • As each composite workitem is executed, it creates it's own children and dispatches them.
  • The one-time teardown for a composite item is scheduled when all it's children have finished.
  • Execution is complete when the top-level item finishes.

New Approach:

  • Prior to creating and dispatching work items, all fixtures will be located and identified as either parallelizable or non-parallelizable.
  • Non-parallel fixtures will be processed separately, one at a time, by adding the single fixture as the top-level item and re-starting the dispatcher shifts for each one of them.
  • Execution is complete when all queues, both fixture and work item, are empty.

Update: The above approach complicated the code too much. Trying a different approach. See following comment.

Note that the above description only solves the specific problem with test fixtures and test cases. For non-parallel setup fixtures, as described in #1474, multiple levels of recursion would be needed since SetUpFixtures may have other SetUpFixtures as well as TestFixtures as children. If the recursion looks simple, I'll try to fix that here as well. Otherwise a separate PR is needed.

@CharliePoole
Copy link
Member Author

Next shot at solving this...

  1. Give WorkItemQueue the ability to save and restore it's inner queue. Coordinate this by putting the save and restore methods in the dispatcher. Figure out how to best deal with concurrency.

  2. When a non-parallel fixture is running, save all the queues, so that the non-parallel fixture has new, empty queues to work with. If possible, do this lazily, so we only actually do the work if it's needed.

  3. When all the queues are empty, check to see if there is a previous set of queues to restore. If so, restore them, if not terminate the run as we now do.

I considered saving a stack of WorkItemQueues, but the problem is that the test workers have a reference to a particular queue. So it seems best to keep the same WIQ and just save the inner queues. @oznetmaster I'd appreciate your thoughts on how best to protect the save and restore methods in terms of concurrency. I'll put in some basic logic that you can comment on.

@CharliePoole
Copy link
Member Author

Reviewing issue #1474, I now believe the PR for this issue will fix it as well. I'm adding a comment there explaining why, so we can discuss whether any more work is needed after PR #2151 is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant