Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Coordinate tests between forks. #334

Closed
schwern opened this Issue · 8 comments

3 participants

@schwern
Owner

Make an option so forked processes will coordinate test details between them, like threads do. Use the issue/334 branch to work on this.

This has to be done completely differently than how threads are done. We'll be drawing heavily off Test::SharedFork. It works like this...

  1. process posts an event
  2. TestState locks their EventCoordiantor storage (create an empty file if necessary)
  3. TestState reads the stored EventCoordinator
  4. TestState uses the new EventCoordinator, discarding its existing one
  5. TestState processes the event
  6. TestState writes their EventCoordinator storage
  7. TestState unlocks storage

See https://github.com/schwern/test-sharedfork/ for some prototype changes we made when trying to patch Test::SharedFork for TB1.5.

Have an object representing the storage

Working with Test::SharedFork demonstrates this makes life so much easier.

Sharing must be explicitly turned on.

Retains backwards compatibility. Its also necessary as we don't want to do the extra work (and bugs) of sharing unless the test forks. The parent must begin sharing events at the point of the fork, so the children can see the events, but the first we'll hear of forking is when a forked process runs a test.

Share the TB2::EventCoordinator

It shares the TB2::EventCoordinator which includes history, formatters and any handlers. Any event or behavior change in any process should be reflected in other processes. This is how most people will want to use it. If the formatter changes, you're going to want it changed everywhere. Only the EventCoordinator is shared, this is due to subtests (see below).

Storage details

Shared event coordinators are frozen and placed into a directory in the temp directory (maybe make this configurable later). Directory and file names are hashed for some security. Make sure permissions are set read/write by owner only.

We're going to hope nobody is silly enough to put their temp directory on a network filesystem that has broken locking. Deal with that problem when it comes.

Each TestState should have its own directory, based on its object_id, to allow multiple states acting in parallel.

Account for subtests

If one process starts a subtest, it should not start one for other processes. This means store TB2::EventCoordinator and not TB2::TestState. This also means each process is looking for a specific stored EventCoordinator, probably by using the object ID.

There can be multiple EC's

Each one representing a subtest. ECs must be stored in such a way that a process can find their EC. This can be accomplished by encoding the object_id in the filename.

Only one EC can be active at a time.

In most formats, subtest output cannot be interlaced with other subtests or its parent. So only one EventCoordinator may be active at a time. When a subtest is started, lock the parent EC. This will block other processes until the subtest is complete.

Use Test::SharedFork's tests

Their tests are really good and get into a lot of subtest edge cases.

Make Test::SharedFork just turn this option on

For Test::Builder 1.5, Test::SharedFork can simply be a wrapper around turning this option on. This makes Test::SharedFork work, and it also provides a 0.x -> 1.5 path for people wanting to write tests with shared forks.

Non-starters

Override fork()

We could override fork() to detect when it happens and begin sharing, but mucking with built ins introduces bugs. And if something else overrides fork our functionality is lost.

CC @tokuhirom and @geistteufel

@schwern
Owner

CC @celogeek if he's interested in taking a crack at this again.

@celogeek

Hi, I'm busy on MooX::Options and validate Dancer2.

Do you want I test again the Plack stuff with the latest version of Test::More ?

@schwern
Owner

@celogeek Not now, thanks. It'll remain broken until Test::SharedFork works. Plack uses Test::TCP which uses Test::SharedFork.

@schwern schwern referenced this issue from a commit
@schwern schwern Copy the Test::SharedFork tests.
We'll adapt them next commit.

For #334
70adbea
@schwern schwern referenced this issue from a commit
@schwern schwern Remove test file numbering.
Ordering generally doesn't matter other than "run first" and "run last".
With a subdirectory it really doesn't matter.

For #334
70fc791
@schwern schwern referenced this issue from a commit
@schwern schwern Convert t/fork/simple.t to Test::More.
* Added logic to check that fork works.
* Make use of d_pseudofork.  It was added in 5.10.0/5.8.9 so we
  still need the fallback logic for backwards compat.
* Removed a redundant counter from the test.  The for loops are
  going to run 20 times no matter what.

For #334
eaceef4
@schwern schwern referenced this issue from a commit
@schwern schwern We're not going to have a fork method.
Redundant test with t/fork/simple.t if we're not going to have a
special fork method.

For #334
d2bec3f
@schwern schwern referenced this issue from a commit
@schwern schwern This threads test makes no sense.
Incomplete threads test from Test::SharedFork.  No need to hang onto
it, though testing forks + threads together would be useful.

For #334
4182ac3
@schwern schwern referenced this issue from a commit
@schwern schwern Remove the Test::SharedFork store tests.
We're likely to do the storage interface differently.

For #334
197db95
@schwern schwern referenced this issue from a commit
@schwern schwern Adapt the remaining Test::SharedFork tests.
They won't run, and I probably made mistakes, but its something to
start from.

For #334
1604c37
@schwern schwern referenced this issue from a commit
@schwern schwern A fork test just using events.
Cut things down to the bare bones.  Should be very useful for
development and debugging.

For #334
c256011
@schwern
Owner

Problem: We can't store the whole Event Coordinator because it contains Formatters which have Streamers which probably have filehandles. There's no sensible way to freeze/thaw an active filehandle so while a fork can inherit a fhilehandle from a parent, it can't go the other way. There's several ways to deal with this...

  • Don't coordinate formatters

This makes a lot of sense from a use-case perspective. If you want your test state coordinated you probably aren't going to be changing formatters or output locations in the middle of the test. This neatly takes care of the problem and is straightforward to implement.

  • Don't coordinate streamers

Filehandles are always going to be in the streamers (or you've written your handler naughty) so don't sync them. This has problems, first is it's difficult to reliably find the streamers as they are inside the formatters. And while TB2::Formatter has a streamer method, there's no guarantee a formatter has only one streamer. TAP has three (output, error and todo). And I can't think of a sensible case where you'd want to change a streamer and not the formatter and want the change to propagate back to the parent process.

  • Create a way to freeze/thaw filehandles.

We could implement something like TB2::ThreadSafeFilehandleAccessor but this approach does not allow changes to filehandles to propagate from child to parent. One approach would be to develop a means to freeze and thaw the state of a filehandle. For special devices this would be storing the fileno... I think. Others would have to store at least the filename, open type and position. Retaining locks would be fascinating.

Of these choices, I'm going to take a crack at not coordinating formatters.

@schwern
Owner

Solved by #367

@schwern schwern closed this
@tokuhirom
@schwern schwern referenced this issue from a commit
@schwern schwern Merge remote-tracking branch 'origin/issue/334' into Test-Builder1.5
Fixes to #334 which came in after issue/334 was merged.
3d92a83
@schwern
Owner
@tokuhirom

Right. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.