Don't use EventEmitter
Opened this issue · 12 comments
Although it's idiomatic Node.js JavaScript it's not present in many other languages. This makes it hard to port the code.
We should replace it with a custom implementation that is easier to port.
For your refactoring pleasure, all files under lib/ and test/ that use EventEmitter:
lib/cucumber/pretty_plugin.jslib/cucumber/sequential_test_case_executor.jslib/cucumber/test_case.jslib/cucumber/test_step.jstest/cucumber/glue_test.jstest/cucumber/pretty_plugin_test.jstest/cucumber/sequential_test_case_executor_test.jstest/cucumber/step_definition_test.jstest/cucumber/test_case_test.jstest/cucumber/test_step_test.js
How do you want to refactor this? With a roll-your-own Observer class?
Not sure yet - need to stare at the code for a while.
This page contains a useful comparison of different Observer patterns (with pros and cons for each):
FWIW, the 2.0 version of Cucumber-Ruby uses a pipes-and-filters setup, where the test cases are fired out of the compiler through a set of filters and finally into the runner. I really like this design as it's stateless, and it feels very modular. In the case of Ruby those filters just have to conform to a duck type / interface with a couple of methods, and we plug them together using the Chain of Responsibility trick - I think @everzet does something similar in Behat too.
FWIW, the 2.0 version of Cucumber-Ruby uses a pipes-and-filters setup, where the test cases are fired out of the compiler through a set of filters and finally into the runner. I really like this design as it's stateless, and it feels very modular.
I may be misunderstanding you, because I still don’t have a super-clear understanding of the architectural big picture—and even less so for the version 2 Cucumber/Gherkin projects. But…
Wasn’t a central goal of the 3.0 version to totally decouple Cucumber and Gherkin?
On one hand, I do like “pipeline” architectures (which, off the top of my head, sounds like the “set of filters” you describe). On the other hand, I want to absolutely certain that the loose-coupling goal is not compromised. I believe the loose coupling is of paramount importance for both the cross-language appeal of Cucumber and Gherkin, as well as their long-term refinement and maintenance.
Now that I’ve blabbered all that, I still don’t know if I understood you correctly. I invite your enlightenment. :)
OK, perhaps I should clarify what I mean by "compiler".
There's a compiler that goes through the gherkin and compiles it into test cases. In Ruby, we fire each of those test cases out down this chain of filters, and finally through a runner.
So Cucumber has a dependency on the test case (or pickles as they've been named in Aslak's latest code). The interface for a filter is three methods:
# example of a pass-through (noop) filter
def with_receiver(receiver)
@receiver = receiver
end
def test_case(test_case)
@receiver.test_case(test_case)
end
def done
@receiver.done
end
So the dependency is on this more abstract type, the test case, rather than on gherkin.
There is an important distinction to make here. There are two different compiler implementations.
There is a compiler in cucumber-ruby which produces test cases from the AST. This design has been refined and reimplemented in gherkin's own compiler (and ported to other languages). This compiler produces pickles.
Cucumber-Ruby doesn't (yet) use gherkin's compiler - it still uses its own.
The goal is to have all Cucumber implementations use the gherkin compiler:
Gherkin Doc--[Gherkin Parser]-->AST--[Gherkin Compiler]-->Pickles--[Cucumber]-->test cases.
This is how we'll move towards a more uniform design across platforms.
For your convenience, here is an expanded reference of the events:
lib/cucumber/test_case.js- (line 5)
'scenario-started'- in
TestCase.execute(eventEmitter) - emitted with
{location: location, pickleSteps: pickle.steps}
- in
- (line 13)
'scenario-finished'- in
TestCase.execute(eventEmitter)(viaTestCase.runTestStepsInSequence(testSteps, eventEmitter, world)) - emitted with
{path: pickle.path, location: location}
- in
- (line 5)
lib/cucumber/pretty_plugin.js- (line 20)
'scenario-started'- in
PrettyPlugin.subscribe(eventEmitter) - listens with
function (scenario) {…}
- in
- (line 25)
'step-started'- in
PrettyPlugin.subscribe(eventEmitter) - listens with
function (step) {…}
- in
- (line 31)
'step-finished': inPrettyPlugin.subscribe(eventEmitter)- listens with
function (step) {…}
- listens with
- (line 20)
lib/cucumber/test_step.js- (line 3)
'step-started'- in
TestStep.execute(world, eventEmitter, run) - emitted with
{status:'unknown', gherkinLocation:…, bodyLocation:…, matchedArguments:…}
- in
- (line 44)
'step-finished':- in
TestStep.execute(world, eventEmitter, run)(viaPromise.then(…)) - emitted with
{status:…, gherkinLocation:…, bodyLocation:…, matchedArgs …}
- in
- (line 3)
Summary
The only events are:
scenario-startedstep-startedstep-finishedscenario-finished
Recommendation
Microcuke is designed to be minimalist and loosely-coupled. Based on the descriptions from the Observer patterns comparison page I listed earlier, I think we should go with the Publish/Subscribe implementation pattern.
I propose we put a simple Pub/Sub “manager” in its own module. Despite the name of the “Publish/Subscribe” pattern, it still fundamentally deals with “events”; therefore, I propose we name the new module events, because I think that name carries the greatest recognition and comfort level for people interested in using microcuke as a reference implementation.
@aslakhellesoy, If you agree with this, I can write up something small and quick for a Pull Request. (If not, please tell me the other considerations or concerns about factoring out EventEmitter.)
@aslakhellesoy ping :)
busy!
k!