Feature idea: Before Fork/ After fork callbacks

[schneems]

I've got a check that ensures my tests didn't leak into my ENV using before and after suite. I needed something similar for checking I'm not leaking docker images as well, but it looks like before and after suite executes at the end of the process rather than when all processes are joined together.

What ends up happening is process 1 finishes while process 2 is still running and hasn't cleaned up yet. The check fires in process 1 and it sees there's a leaked image, even though it will be cleaned up before process 2 exits. If I could delay the check until all processes are finished, then I can get rid of my race condition.

[grosser]

should be doable since we control the workers directly, PR welcome, but I'm most likely not going to work on that since I don't have a use for it ...

[schneems]

I've been looking into implementing this and I wanted to share my notes/findings. I don't consider this a feature proposal yet. I'm curious to hear your high-level thoughts on these findings. I'm looking for any potential alternatives you could think of for implementation. I'm also looking for a temperature reading of how 👍 👎 you might be on something like this example implementation.

Problem with implementing before/after split semantics

To be able to enable before/after semantics, we need to read that code into memory. Usually, people expect to put their test config into spec_helper.rb. The problem is, this file won't be read until after the code has already forked (inside of a Parallel.in_process ) link.

This blocks an easy addition of before_fork semantics. I tried adding an after_parallel_test block but ran into a similar issue. Because the parent process never loads the rspec code at all, it doesn't know what's in spec_helper.rb, so it couldn't invoke any user-specified code.

Extra conditions

In addition to being able to have before/after semantics, I also want any interface for parallel_split_test to degrade gracefully. Meaning if someone calls rspec on the test suite (rather than parallel_split_test), then I want them to be able to run the suite and have any before/after split semantics gracefully degrade into before/after :suite semantics.

A viable workaround needs

To have all these conditions met we need:

• A: A way that parallel_split_test cli can load code into the parent process before reading in the test suite or forking.
• B: A way that parallel_split_test can communicate internally if code is being loaded by the parallel_split_test cli or not (to feature switch when being invoked via rspec instead of parallel_split_test).
• C: An interface to define before/after split such as ParallelSplitTest.before_split(&block).
• D: A way to degrade before/after split functionality when plain rspec` is being invoked (Condition B is false).

Possible way forward

• A: A way that parallel_split_test cli can load code into the parent process before reading in the test suite or forking.
  • Introduce a spec/parallel_split_config.rb that can be read by the CLI without invoking tests.
• B: A way that parallel_split_test can communicate internally if code is being loaded by the parallel_split_test cli or not (to feature switch when being invoked via rspec instead of parallel_split_test).
  • Add a predicate ParallelSplitTest.running_in_parallel? predicate that is set to true only when the parallel_split_test cli is used.
• C: An interface to define before/after split such as ParallelSplitTest.before_split(&block).
  • Add the interface
• D: A way to degrade before/after split functionality when plain rspec` is being invoked (Condition B is false).
  • Have the DSL for the config file auto degrade to use Rspec.configure.before(:suite) when ParallelSplitTest.running_in_parallel? is false.
  • Require the file spec/parallel_split_config.rb from spec_helper.rb so it's still invoked even when not being found via parallel_split_test

For example

Inside of spec_helper.rb add a:

require_relative "parallel_split_config.rb"

This way even when the tests are run with rspec, they'll know about the before/after hooks we need.

Inside of the file could look like this:

require "parallel_split_test"
require "cutlass"

JVM_BUILDPACK = Cutlass::LocalBuildpack.new(directory: test_dir.join("meta-buildpacks/java"))
ParallelSplitTest.before_split do
  JVM_BUILDPACK.setup
end

ParallelSplitTest.after_split do
  JVM_BUILDPACK.teardown
end

If this is invoked multiple times it only runs on first invocation (due to require_relative semantics). Forked processes should have access to this memory.

Running cases with/without parallel_split_test:

• With parallel split test:
  • PST sees this file exists, reads it in first, then runs before_split action, then does the regular runner logic, then runs any after_split actions.
• Without parallel split test:
  • Rspec sees the require, loads the code. The ParallelSplitTest.before_split method can check to see if it's being invoked inside of a PST runner. If not, then degrade to Rspec.configure.before(:suite) so the code still gets executed.

Alternatives

We could perhaps default to making that "special config file" simply spec_helper.rb. I believe all of mine can be required out of the box. The cli flag could be written as something more generic like --parent-require-before-fork=spec/spec_helper.rb. The downside is that I don't think we could make this the default behavior since it might break people's suites who aren't using this feature/functionality.

Analysis

It's a bit more round-about than I was originally hoping for (interface-wise). But the main process not reading in the rspec code at all is a fairly major constraint on configuration options. This is the only way I can find to satisfy all my requirements. We could move logic around a bit, but I think the need to read in a stand-alone file is strong.

I would say my overall temperature on this implementation is somewhat cool at this moment in time. I think it would work and satisfy my criteria but it seems like it could be difficult to understand what's being done and why when there's a bug in the test setup/teardown.

[grosser]

Alternative idea:
I had ok experience with loading test helper early when building https://github.com/grosser/forking_test_runner
there are a few edge-cases but it generally worked.
It should also make things faster since we can avoid duplicate loading.
The before/after hook would then be "execute immediately if not using cli" or "execute before/after fork".
... but it would break any workflow that relies on TEST_ENV_NUMBER bing different during boot, which would be common for rails apps :/
Tt might work, but would have to be a big rewrite / change of how things work and lead to lots of fallout.

Extra config file: sounds like a much simpler approach and would allow opt-in for people that need the feature, could be something that can be easily merged and played with, so I'd appreciate a PR that explores this.