Skip to content
This repository has been archived by the owner on May 28, 2019. It is now read-only.

Don't use EventEmitter #8

Open
aslakhellesoy opened this issue Feb 9, 2016 · 12 comments
Open

Don't use EventEmitter #8

aslakhellesoy opened this issue Feb 9, 2016 · 12 comments

Comments

@aslakhellesoy
Copy link
Contributor

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.

Ref cucumber/common#8 (comment)

@Zearin
Copy link

Zearin commented Feb 11, 2016

For your refactoring pleasure, all files under lib/ and test/ that use EventEmitter:

  • lib/cucumber/pretty_plugin.js
  • lib/cucumber/sequential_test_case_executor.js
  • lib/cucumber/test_case.js
  • lib/cucumber/test_step.js
  • test/cucumber/glue_test.js
  • test/cucumber/pretty_plugin_test.js
  • test/cucumber/sequential_test_case_executor_test.js
  • test/cucumber/step_definition_test.js
  • test/cucumber/test_case_test.js
  • test/cucumber/test_step_test.js

@Zearin
Copy link

Zearin commented Feb 11, 2016

How do you want to refactor this? With a roll-your-own Observer class?

@aslakhellesoy
Copy link
Contributor Author

Not sure yet - need to stare at the code for a while.

@Zearin
Copy link

Zearin commented Feb 15, 2016

This page contains a useful comparison of different Observer patterns (with pros and cons for each):

https://github.com/millermedeiros/js-signals/wiki/Comparison-between-different-Observer-Pattern-implementations

@mattwynne
Copy link

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.

@Zearin
Copy link

Zearin commented Feb 18, 2016

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. :)

@mattwynne
Copy link

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.

@aslakhellesoy
Copy link
Contributor Author

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.

@Zearin
Copy link

Zearin commented Feb 29, 2016

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}
    • (line 13) 'scenario-finished'
      • in TestCase.execute(eventEmitter) (via TestCase.runTestStepsInSequence(testSteps, eventEmitter, world))
      • emitted with {path: pickle.path, location: location}
  • lib/cucumber/pretty_plugin.js
    • (line 20) 'scenario-started'
      • in PrettyPlugin.subscribe(eventEmitter)
      • listens with function (scenario) {…}
    • (line 25) 'step-started'
      • in PrettyPlugin.subscribe(eventEmitter)
      • listens with function (step) {…}
    • (line 31) 'step-finished': in PrettyPlugin.subscribe(eventEmitter)
      • listens with function (step) {…}
  • lib/cucumber/test_step.js
    • (line 3) 'step-started'
      • in TestStep.execute(world, eventEmitter, run)
      • emitted with {status:'unknown', gherkinLocation:…, bodyLocation:…, matchedArguments:…}
    • (line 44) 'step-finished':
      • in TestStep.execute(world, eventEmitter, run) (via Promise.then(…))
      • emitted with {status:…, gherkinLocation:…, bodyLocation:…, matchedArgs …}

Summary

The only events are:

  • scenario-started
  • step-started
  • step-finished
  • scenario-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.)


@Zearin
Copy link

Zearin commented Mar 16, 2016

@aslakhellesoy ping :)

@aslakhellesoy
Copy link
Contributor Author

busy!

@Zearin
Copy link

Zearin commented Mar 16, 2016

k!

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

No branches or pull requests

3 participants