Make function (actor) => Promise a first-class Performable #23

InvictusMB · 2017-02-05T22:12:02Z

Closes #21, #22

Actor is now able to attemptTo functions like (actor) => Promise
Task decoration is decoupled from notification
Notification relies on StepAnnotationSymbol which makes it easy to decorate functions
Task execution is delegated from Actor to StepExecutor who manages StepNotifier

Concerns:

Actor now depends on StageManager which might be undesirable. But I found no way to work around this. Whoever executes the Performable has to deal with notification as Performable is no longer bundled with notification.
StepExecutor ended up in recording namespace. It kind of makes sense as it is tightly coupled with StepNotifier but still looks awkward to me
There are many entities with Step in name but in fact they all deal with Performables. I tried to follow naming patterns but it feels uncomfortable. Is step name for decorator a legacy?

…rom step decoration

…ctions

coveralls · 2017-02-05T22:24:52Z

Coverage decreased (-0.9%) to 93.502% when pulling f5e8e22 on InvictusMB:performables into dca8be0 on jan-molak:master.

coveralls · 2017-02-05T23:01:06Z

Coverage decreased (-0.9%) to 93.519% when pulling 3611b72 on InvictusMB:performables into dca8be0 on jan-molak:master.

jan-molak · 2017-02-06T00:03:54Z

Hey, thanks for this.

To answer your question, yes @step is a bit of a legacy; it was there to keep the convention the same between Serenity BDD and the JS version and help Java teams transitioning to a new language.
On the same note - a Performable should really be an Activity and the Activity should be some ActivityIdentifier or something along those lines; this then works better with ActivityStarted/Finished notifications and is more consistent with the system metaphor. But that's for another refactoring once 1.0 is out.

After our conversation (#21) I'm thinking of making some more changes it this space too.

Here are my initial thoughts:

leave the @step as it is for backwards compatibility reasons
add a class-level @reported annotation, which decorates the performAs to issue the ActivityStarted/ActivityFinished events
use toString, if provided on the Task; this seems cleaner than a parameter on the annotation. If toString is not provided we can always try to convert the name of the class to somethings human-readable -> "James attempts to add a todo item" for AddATodoItem class.

So instead of:

export class Start implements Task {

    static withATodoListContaining(items: string[]) {
        return new Start(items);
    }

    @step('{0} starts with a Todo List containing #items') 
    performAs(actor: PerformsTasks): PromiseLike<void> {
        return actor.attemptsTo(
            Open.browserOn('/examples/angularjs/'),
            ...this.items.map(item => AddATodoItem.called(item))
        );                                                    
    }

    constructor(private items: string[]) {
    }

    private addAll(items: string[]): Task[] {     
        return items.map(item => AddATodoItem.called(item)); 
    }
}

we'd have:

@reported
export class Start implements Task {

    static withATodoListContaining = (items: string[]) => new Start(items);

    performAs = (actor: PerformsTasks) => actor.attemptsTo(
        Open.browserOn('/examples/angularjs/'),
        ...this.addAll(this.items)                          
    );                                                    

    toString = () => '{0} starts with a Todo List containing #items';

    constructor(private items: string[]) {
    }
}

Those changes, together with favouring arrow functions over the more traditional syntax would help with:

reducing the amount of code required to implement a task
maintaining the tooling support (IntelliSense, refactoring, etc)

Which I think are both worthwhile.

On the "funcitonal note"; I'm not sure how I feel about the Actor being coupled with the StepExecutor... We could deploy the heavy machinery of DI on it, but I'm wondering if it's really necessary.

First of all, I like the below style, because it both accomplishes your original goal and should help the IDEs understand the code in the non-TypeScript scenario:

const PayWithCreditCard = {
    number(creditCardNumber: string) {
        return describeStep(
            actor => actor.attemptsTo( /*...*/ ),
            `{0} pays with a credit card number ${creditCardNumber}`,
        );
    },
};

We could take it even further:

const PayWithCreditCard = {
    number: (creditCardNumber: string) => describeStep(
        actor => actor.attemptsTo( /*...*/ ),
        `{0} pays with a credit card number ${creditCardNumber}`,
    ),
};

Or to make it read even nicer:

const PayWithCreditCard = { number: (creditCardNumber: string) => 
    aTaskWhere(`{0} pays with a credit card number ${creditCardNumber}`,
        actor => actor.attemptsTo( /*...*/ )
    ),
};

Or even skip passing the functions, and pass lists of things to do instead:

const PayWithCreditCard = { number: (creditCardNumber: string) => 
    aTaskWhere(`{0} pays with a credit card number ${creditCardNumber}`,
      task1,
      task2,
      task3
    ),
};

Now, since there are no annotations available to us in the JavaScript land, could the describeStep, a.k.a. aTaskWhere return some ReportedTask? Basically, a default implementation of the task class takes a description of the task and a list of things to do.

For example:

@reported                           // that would be in serenity core, so annotations are allowed
class ReportedTask implements Task {

    constructor(private description: string, private tasks: Task[]) {
    }

    performAs = (actor: PerformsTasks) => actor.attemptsTo(...this.tasks);

    toString = () => this.description;
}

we could even call it Tasks to keep it simple:

@reported
class Tasks implements Task {

    constructor(private description: string, private tasks: Task[]) {
    }

    performAs = (actor: PerformsTasks) => actor.attemptsTo(...this.tasks);

    toString = () => this.description;
}

and:

export function aTaskWhere(description: string, ...tasks: Task[]) {
  return new Tasks(description, tasks);
}

Would that work? It's late though, so I might be missing something :-)

What I'm proposing is not really a functional interface, more of a builder to help with encouraging task composition, which also seems to accomplish the goal of working in JavaScript, JavaScript/Flow and TypeScript environments.

Come to think about it, it works even for single-element "todo lists", such as:

const Submit = aTaskWhere(`{0} submits the form`,
  Click.on(Form.Submit_Button)
);

What do you think?

InvictusMB · 2017-02-06T09:25:54Z

I like these ideas and I think everything you mentioned is doable on top of this PR.

So that

step decorator will attach StepExecutor and StepNotifier to a Performable and describeStep will do the same with FunctionalPerformable. Therefore Actor will no longer be coupled with StageManager
aTaskWhere can be turned into a simple task factory to do

const Submit = aTask(
  Click.on(Form.Submit_Button)
).where(
  `{0} submits the form`)
);

Something like

export function aTask(...tasks: Attemptable[]): Describable {
    return toDescribable(compose(...tasks));
}

export function toDescribable(performable: FunctionalPerformable): Describable {
    return extend<Describable>(performable, {
        where: (descriptionTemplate) => describeStep(performable, descriptionTemplate)
    });
}

export function compose(...tasks: Attemptable[]) : FunctionalPerformable {
    return actor => actor.attemptsTo(...tasks);
}

export interface Describable extends FunctionalPerformable {
    where(descriptionTemplate: string): FunctionalPerformable;
}

I will give it a try this evening.

I'm no longer sure if fluent task builder from #21 will add much value on top of this API. It's quite powerful already. Fluent stuff was more meant to abstract away Task construction. Maybe now it can be focused only on constructing a DSL for a Task and not on composition and annotation?
I would narrow the scope of #21 to Fluent Task Builder to keep that discussion separate from 'functional vs OOP topic'.

InvictusMB · 2017-02-07T22:22:09Z

@jan-molak
Ok, I was able to implement this.

Now functional tasks look even more compact

const PayWithCreditCard = {
    number: (creditCardNumber: string) => aTask()
        .where(`{0} pays with a credit card number ${creditCardNumber}`)
};

const PlayAChord = {
    called: (chord: Chord) =>
        actor => PlayAnInstrument.as(actor).play(chord),
};

const playThe = chords =>
    chords.map(chord => PlayAChord.called(chord));

const PerformASong = (musicSheet: MusicSheet) => ({
    performAs: aTask(...playThe(musicSheet.chords)),
});

Autocompletion in IntelliJ also works well with aTask factory and all the chaining. So it should be possible to annotate with types even dynamically constructed fluent interfaces.

- `Performable` is really an `Activity` that an `Actor` can perform, therefore it should be called as such. - What used to be an `Activity` is now a `RecordedActivity`. `RecordedActivity` is just a "tiny type", that captures the name of the real `Activity` and works alongside with `Outcome` to capture the result and other meta data related to the performance. - To make the model consistent, `Scene` is now a `RecordedScene` - To help distinguish `RecordedActivity` related to a `Task` and `Interaction`, the `@step()` annotation can now take an optional second arugment `ActivityType.Task` or `ActivityType.Interaction`. A `@step` that doesn't specify its type is considerd to be of type `Task` by default as 90% of time you'll be writing Tasks not Interactions. - The distinction between a `RecordedTask` and `RecordedInteraction` will help to make the configuration of the `Photographer` more fine-grained, so that we could for example tell it to only capture screenshots of interactions rather than all the tasks. This will also help to enable the granularity needed for #18. - `See` and `CompareNotes` are now `Interactions` rather than generic `Activities` (pretending to be `Tasks`), as performing an assertion is also an interaction with the system. No need for special treatment of those two classes. - The `Interaction` interface is also corrected to allow for `Actors` that `UseAbilities` but also `AnswerQuestions` Those changes should not affect the consumers of the Serenity/JS APIs and will help tackle the tech debt before it spreads ;-) Enables #18, #22, #23

jan-molak · 2017-02-12T22:03:28Z

Apologies for making the PR conflict with the master, but I wanted to clean up the domain model before incorporating the new features.

InvictusMB · 2017-02-13T07:25:14Z

No problem. It has to be refined anyway.
I will get back to this next week.

InvictusMB · 2017-03-12T10:29:45Z

@jan-molak
I finally got some time to get this moving again. But there are a few things I would like to clarify first.

I don't quite get the value of ActivityType enum. I see the intention to have different handling from Photographer perspective. But I would rather have an explicit flag telling to capture or not to capture an activity and not infer this from an activity type. Or I would create a Capturable interface. As I can imagine both Capturable and non-Capturable Tasks and Interactions.
I don't fully understand the way the split between Interactions and Tasks is done. I see that Interaction is a lower level abstraction of an Actor activity and a Task is a higher level abstraction with composition in mind.
I also see that Interactions depend on Abilities while Tasks do not. And this is where I don't see a reason for segregation. I can as well imagine a Task that would require an ability.

So why not call everything an Activity? And make certain Activities implement RequiresAbility interface if they depend on Ability. That would greatly simplify mental model for me.
Then the Task/Interaction split would be a question of abstraction level and not related to dependency on Abilities.
I could also argue that an Ability is an Activity with some predefined state. And then you could reason about an Ability as a way to do DI of Activities into other Activities.

And finally AnswersQuestions interface looks to me like an Actor bundled with an Ability to AnswerQuestion which requires Ability to See. But AnswerQuestion might also use an Ability to Know if he has Notes, right?
So AnswersQuestions could be

export interface AnswersQuestions {
    toSee<T>(question: Question<T>): T;
    toKnow<T>(question: Question<T> | Topic): T;
}

jan-molak · 2017-03-12T20:05:43Z

Hey @InvictusMB, it's great to hear from you again!

I don't quite get the value of ActivityType enum. I see the intention to have different handling from Photographer perspective.

The idea behind the ActivityType is to work around the interface erasure that happens when TypeScript is transpiled to JavaScript and there's no easy way to find out whether we're working with an Interaction or a Task anymore. There's a way, I think, to emit the interface metadata and then use some reflection techniques to get it back, but I didn't have a chance to look into it in more detail.

But I would rather have an explicit flag telling to capture or not to capture an activity and not infer this from an activity type. Or I would create a Capturable interface. As I can imagine both Capturable and non-Capturable Tasks and Interactions.

Yes, that was my first impulse too, but it becomes more complicated when we start sharing Activites as npm modules between teams. What's worth capturing from one team's perspective, may not be of interest to another, so if we bake the "capture-worthiness" of an activity into its definition it loses on portability.

I don't fully understand the way the split between Interactions and Tasks is done. I see that Interaction is a lower level abstraction of an Actor activity and a Task is a higher level abstraction with composition in mind. I also see that Interactions depend on Abilities while Tasks do not. And this is where I don't see a reason for segregation. I can as well imagine a Task that would require an ability.

The difference is that an Interaction can change the state of the system while a Task on its own can't.

Because of this split, we can limit the the actions of some of the stage crew members, such as the Photographer, to only capture screenshots (or the HTML of the page, etc.) when the state of the system changes, so when an interaction occurs.

So why not call everything an Activity? And make certain Activities implement RequiresAbility interface if they depend on Ability. That would greatly simplify mental model for me.
Then the Task/Interaction split would be a question of abstraction level and not related to dependency on Abilities.

I'm not sure if we can achieve the above behaviour of the stage crew members if all the activities looked the same?

I could also argue that an Ability is an Activity with some predefined state. And then you could reason about an Ability as a way to do DI of Activities into other Activities.

Interesting, could you please give an example of what you have in mind?

And finally AnswersQuestions interface looks to me like an Actor bundled with an Ability to AnswerQuestion which requires Ability to See. But AnswerQuestion might also use an Ability to Know if he has Notes, right?

True, it might.

So AnswersQuestions could be

export interface AnswersQuestions {
    toSee<T>(question: Question<T>): T;
    toKnow<T>(question: Question<T> | Topic): T;
}

Yeah, I guess it could. Although it could also become:

export interface AnswersQuestions {
    toAnswer<T>(question: Question<T>): T;
}

And then we can have questions that require the actor to be able to see something on the screen, to read their notes, to check the last received HTTP response and so on.

InvictusMB · 2017-03-12T21:35:58Z

The idea behind the ActivityType is to work around the interface erasure that happens when TypeScript is transpiled to JavaScript and there's no easy way to find out whether we're working with an Interaction or a Task anymore.

But should we know the type? Relying on feature detection makes more sense to me than using types. Especially in JS world.

There's a way, I think, to emit the interface metadata and then use some reflection techniques to get it back, but I didn't have a chance to look into it in more detail.

Symbols work quite well there. They are unique and immutable so that you can get data back only if you have a reference to the symbol you used to set a property. I used this technique to attach metadata to functional Tasks.

The difference is that an Interaction can change the state of the system while a Task on its own can't.

Because of this split, we can limit the the actions of some of the stage crew members, such as the Photographer, to only capture screenshots (or the HTML of the page, etc.) when the state of the system changes, so when an interaction occurs.

I would argue with this using the same appeal to portability. I might say that for some domains I would want to treat a particular Task as an atomic transaction.
Let's say I have a FillTheForm task. And I only want to know the state of the system before and after filling all fields of the form. And I want a Photographer to respect that and not to capture intermediate screenshots to reduce noise.

I might also want that to differ on per-scenario basis. So that in scope of FillPaymentDetails scenario I'm interested in all Interactions with each form field. But in a bigger picture when I consider MakePurchase scenario I want a FillPaymentDetails task to be atomic and not report internal steps.

So from my perspective an Activity might be a Task in one context but become an Interaction when I move up in abstraction levels.

I could also argue that an Ability is an Activity with some predefined state. And then you could reason about an Ability as a way to do DI of Activities into other Activities.

Interesting, could you please give an example of what you have in mind?

For example, Memoize interaction that requires TakeNotes Ability. While TakeNotes Ability may store Notes in memory or may persist them in a file or DB.
So there may be two actors of which first would TakeNotes in memory and second TakeNotes to file. But for Memoize interaction the difference is irrelevant.
In essence it's just an Ability being configured and injected into Interaction through Actor.

But then we could take it further. Imagine a Task Pay.with(PayPalAccount) that fills the PayPal form. But in a different context I can as well consider this an ability. So when I want to have a MakePayment interaction that would require a Pay ability I could create an Actor.whoCan(Pay.with(PayPalAccount)) and another one Actor.whoCan(Pay.with(CreditCard)). And both of them would be able to do MakePayment interaction.

Which makes differences between Task, Interaction and Ability appear and disappear depending on context and abstraction level we want to use them in.
Does that make sense or am I overengineering?

InvictusMB · 2017-03-13T09:59:04Z

After a few iterations on Screen Play pattern I have the following understanding:

An Interaction is an atomic Activity that causes the change in system state.
A Task is a complex Activity that may be a composition of other Activities.
An Ability is statement of possibility for an Actor to execute an Activity.
An Activity can demand from an Actor some Abilities in order to succeed.

So from automation engineer perspective I want to have this:

A Pay.with(PayPalAccount) Activity.
An Actor.whoCan(Pay.with(PayPalAccount))
A MakePurchase Activity that will require a Pay Ability from an Actor.

Which might look like this:

function MakePurchase(product) {
  return actor => actor.attemtsTo(
    GoTo(OffersPage),
    Select(product),
    UseAbility.to(Pay),
    TakeNotes(getPurchaseId)
  );
}

And ultimately the scenario:

Actor.whoCan(Pay.with(PayPalAccount))
  .attemtsTo(MakePurchase(aBook))

jan-molak · 2017-03-13T11:01:26Z

An Interaction is an atomic Activity that causes the change in system state.

That's right

A Task is a complex Activity that may be a composition of other Activities.

That's correct, I'd also add that a Task doesn't necessarily have to cause a change in the system state. It could ask a question, for example.

An Ability is statement of possibility for an Actor to execute an Activity.

That's a nice definition :-) Yes, you can also think about the Ability as a client to some external interface of the system. So BrowseTheWeb is a client that understands how to use the web interface, UseTheAPI could talk to some HTTP endpoint, and so on.

An Activity can demand from an Actor some Abilities in order to succeed.

An Interaction, yes, a Task - not necessarily.

So from automation engineer perspective I want to have this:

A Pay.with(PayPalAccount) Activity.

Did you mean "Ability" or "Activity"?

An Actor.whoCan(Pay.with(PayPalAccount))

A MakePurchase Activity that will require a Pay Ability from an Actor.

Which might look like this:

function MakePurchase(product) {
  return actor => actor.attemtsTo(
    GoTo(OffersPage),
    Select(product),
    UseAbility.to(Pay),       // (1)
    TakeNotes(getPurchaseId)  // (2)
  );
}

How would the UseAbility.to(Pay) know how to actually make the payment? Enter the PayPal credentials, confirm the purchase, etc?
I'm guessing that getPurchaseId would be a function that reads the state?

And ultimately the scenario:

Actor.whoCan(Pay.with(PayPalAccount))
  .attemptsTo(MakePurchase(aBook))

Yup, that looks cool.

By the way, with the recent introduction of Lerna, you could create the functional Screenplay as a new npm module that could be used instead of the OO version.

InvictusMB · 2017-03-13T16:02:54Z

So from automation engineer perspective I want to have this:

A Pay.with(PayPalAccount) Activity.

Did you mean "Ability" or "Activity"?

In this context I did mean Activity. Which would become an Ability if used in context of Actor.whoCan().

How would the UseAbility.to(Pay) know how to actually make the payment? Enter the PayPal credentials, confirm the purchase, etc?

That's the crucial point of my reasoning. UseAbility.to(Pay) will assert that an Actor was configured with an instance of Pay Activity as in Actor.whoCan(Pay.with(PayPalAccount)) and will execute that Activity. This will allow me to reuse same MakePurchase Activity in different scenarios with different payment options.

I'm guessing that getPurchaseId would be a function that reads the state?

Yes, getPurchaseId could be a Question to execute on system.

An Activity can demand from an Actor some Abilities in order to succeed.

An Interaction, yes, a Task - not necessarily.

Abstracting from a current codebase but applying a formal logic, a Task requires a particular Ability if any of underlying Interactions require that Ability, doesn't it?

By the way, with the recent introduction of Lerna, you could create the functional Screenplay as a new npm module that could be used instead of the OO version.

Good point. I will try to do it that way.

jan-molak · 2017-03-13T16:20:30Z

So from automation engineer perspective I want to have this:

A Pay.with(PayPalAccount) Activity.

Did you mean "Ability" or "Activity"?

In this context I did mean Activity. Which would become an Ability if used in context of Actor.whoCan().

How would the UseAbility.to(Pay) know how to actually make the payment? Enter the PayPal credentials, confirm the purchase, etc?

That's the crucial point of my reasoning. UseAbility.to(Pay) will assert that an Actor was configured with an instance of Pay Activity as in Actor.whoCan(Pay.with(PayPalAccount)) and will execute that Activity. This will allow me to reuse same MakePurchase Activity in different scenarios with different payment options.

I'm not sure if I can fully visualise how that would work just yet, it might be easier to discuss it over a prototype perhaps?

An Activity can demand from an Actor some Abilities in order to succeed.

An Interaction, yes, a Task - not necessarily.

Abstracting from a current codebase but applying a formal logic, a Task requires a particular Ability if any of underlying Interactions require that Ability, doesn't it?

Yes, a Task will have a transitive dependency on Ability or several Abilities, but it can be via both Interactions and Questions.

By the way, with the recent introduction of Lerna, you could create the functional Screenplay as a new npm module that could be used instead of the OO version.

Good point. I will try to do it that way.

Cool! It would be nice to make the mechanism pluggable eventually.

…ng instead of @step affects: serenity-js As per our conversation in #23, ActivityType turned out to not add much value, so it's now removed. As RecordedScenes and RecordedActivities now contain information about the place where they've been invoked, it should be possible to implement a better filtering mechanism based on paths.

InvictusMB · 2017-04-29T15:49:09Z

Closing this as no longer relevant since #21 and #22 are now resolved.

InvictusMB added 8 commits February 5, 2017 16:24

feat(tasks): Make it possible for actor to execute plain function tasks

55df368

feat(tasks): Extract notification of stage manager responsibilities f…

74b60f0

…rom step decoration

feat(tasks): Make notifications actor's responsibility

03eb114

feat(tasks): Make it possible to describe functional tasks and intera…

f09ab71

…ctions

feat(tasks): Extract task execution and notification logic from actor

f818eb4

feat(tasks): Move step executor to recording namespace

3353ba2

feat(tasks): Extract step executor and step notifier into separate files

7790774

feat(tasks): Improve naming

f5e8e22

feat(tasks): Take StageManager out of Actor constructor parameters

3611b72

InvictusMB added 2 commits February 7, 2017 22:50

feat(tasks): Decouple actor from notifications

e72288f

feat(tasks): Use aTask for composition of functional tasks

67e78a6

InvictusMB closed this Apr 29, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make function (actor) => Promise a first-class Performable #23

Make function (actor) => Promise a first-class Performable #23

InvictusMB commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

jan-molak commented Feb 6, 2017 •

edited

InvictusMB commented Feb 6, 2017 •

edited

InvictusMB commented Feb 7, 2017

jan-molak commented Feb 12, 2017

InvictusMB commented Feb 13, 2017

InvictusMB commented Mar 12, 2017 •

edited

jan-molak commented Mar 12, 2017 •

edited

InvictusMB commented Mar 12, 2017 •

edited

InvictusMB commented Mar 13, 2017 •

edited

jan-molak commented Mar 13, 2017 •

edited

InvictusMB commented Mar 13, 2017

jan-molak commented Mar 13, 2017 •

edited

InvictusMB commented Apr 29, 2017

Make function (actor) => Promise a first-class Performable #23

Make function (actor) => Promise a first-class Performable #23

Conversation

InvictusMB commented Feb 5, 2017

coveralls commented Feb 5, 2017

coveralls commented Feb 5, 2017

jan-molak commented Feb 6, 2017 • edited

InvictusMB commented Feb 6, 2017 • edited

InvictusMB commented Feb 7, 2017

jan-molak commented Feb 12, 2017

InvictusMB commented Feb 13, 2017

InvictusMB commented Mar 12, 2017 • edited

jan-molak commented Mar 12, 2017 • edited

InvictusMB commented Mar 12, 2017 • edited

InvictusMB commented Mar 13, 2017 • edited

jan-molak commented Mar 13, 2017 • edited

InvictusMB commented Mar 13, 2017

jan-molak commented Mar 13, 2017 • edited

InvictusMB commented Apr 29, 2017

jan-molak commented Feb 6, 2017 •

edited

InvictusMB commented Feb 6, 2017 •

edited

InvictusMB commented Mar 12, 2017 •

edited

jan-molak commented Mar 12, 2017 •

edited

InvictusMB commented Mar 12, 2017 •

edited

InvictusMB commented Mar 13, 2017 •

edited

jan-molak commented Mar 13, 2017 •

edited

jan-molak commented Mar 13, 2017 •

edited