Make function a first-class Performable

[InvictusMB]

Make function a first-class Performable

[jan-molak]

Those are some good points, and thanks for taking your time to raise them and provide an example implementation!

There is too much cognitive load to figure out what the task does. You have to find the needed class, then scan the class for performAs method and then find what the actor does. At the end you already forgot where you started and why you where doing that.

The name of the task should be descriptive enough so that its purpose (the what) is easy to understand - AddAnItem, BookAFlight, MakeAPayment, ChooseADefaultCard, etc.
The inner-workings of the task (the how) are an implementation detail.

Could you please share an example where it's necessary to look for the performAs to find out what the task does?

Private methods make it easy to deviate from single responsibility and stuff the class with everything you can think of instead of doing proper composition.

Yes, that could be a risk. Did you find it was happening during your POC? If so, could you please share some examples so that I can understand the circumstances better?

Static methods look like a boilerplate only to provide a fluent DSL for Task. In most cases they can be combined and DRY'ed out to Map<setter, propertyName>. This also adds up to cognitive load when reviewing tasks.

Well, kind of. One important thing that static methods provide is an information to the IDE about the API a given task has. I found that this makes writing test scenarios much faster because in tools like WebStorm or IntelliJ you can just start writing the name of the task, ctrl+space and you know what a given task offers, even without having to look at its source code.

---

Having said that, I like the idea of functions being first-class tasks. I also agree it could feel more natural to JavaScript. However, there's a couple of things I think we should be mindful of.

Example

Let's consider the following example:

actor.attemptsTo(
  BookAReturnFlight.
      from('London').
      to('Copenhagen').
      leavingOn('2017-02-10').
      returningOn('2017-02-12').
      inStandardEconomy().
      withPriorityBoarding()
);

The above task is a builder putting together tasks to:

• open the app
• navigate to flight booking
• choose the origin airport - London
• choose the destination airport - Copenhagen
• choose the departure date - 2017-02-10
• choose the return date - 2017-02-12
• choose the class - standard economy
• add priority boarding - priority boarding

Some of those tasks can have nested tasks, for example:

• choose the departure date - 2017-02-10
  • click the calendar widget
  • select year - 2017
  • select month - 02
  • select day - 10
  • click on some button

If I understand the example you presented correctly, we'd have something along the lines of:

export const BookAReturnFlight =
  defineTask()
    .addActions(
      openTheApp,
      openFlightBooking,
      chooseOriginAirport,
      chooseDestinationAirport,
      chooseDepartureDate,
      chooseReturnDate,
      chooseClass,
      addPriorityBoarding,
    )
    .annotate('{0} books a #flightClass flight from #origin to #destination, ' +
             'leaving on #departureDate and returning on the #returnDate #withPriorityBoarding')
    .defineSetters({
      from: 'origin',
      to: 'destination',
      leavingOn: 'departureDate',
      returningOn: 'returnDate',
      inStandardEconomy: 'economyClass',
      withPriorityBoarding: 'priorityBoarding'      
    });

Questions:

1. Complex tasks can have a number of parameters, as per the above example. If we're going with a task builder like defineTask, how can we:
   • 1.1. make sure that there are no parameter name collisions? Several tasks could be easily using the same name for their parameter; how would defineSetters know which task we have in mind?
   • 1.2. make it obvious which task requires what parameter so that devs don't have to look for its source (encapsulation)?
   • 1.3. how do we handle generating the annotation for the boolean fields, like priorityBoarding? We'd still need some sort of a method/function generating a string representation of the flag, would we not?
2. Another thing is the tooling support, which I found of high importance. Modern IDEs still have pretty bad support for JavaScript - basically, they have no way of figuring out how the IntelliSense should work in a dynamic language, such as JS. Could this point be addressed with dynamic tasks builders?

Looking forward to hearing your thoughts!
Jan

[jan-molak]

Thinking about it, 1.1 and 1.2 could be addressed by making the parameters of the builder explicit, for example:

export const BookAReturnFlight = aTaskTo( (origin, destination, departureDate, /* etc. */) => {
      openTheApp(),
      openFlightBooking(),
      chooseOriginAirport(origin),
      chooseDestinationAirport(destination),
      chooseDepartureDate(departureDate),
      // etc.
    })
    .where('{0} books a #flightClass flight from #origin to #destination, ' +
             'leaving on #departureDate and returning on the #returnDate #withPriorityBoarding');

I'm still not sure how to nicely generate the DSL methods so that:

• they make sense in the context, i.e. instead of setPriorityBoarding(false) you'd get withPriorityBoarding()
• the tooling support is as good as with the static methods (this might not be possible to accomplish, though, which would be a shame)

Thoughts?

[InvictusMB]

The name of the task should be descriptive enough so that its purpose (the what) is easy to understand - AddAnItem, BookAFlight, MakeAPayment, ChooseADefaultCard, etc.
The inner-workings of the task (the how) are an implementation detail.
Could you please share an example where it's necessary to look for the performAs to find out what the task does?

Code reviewing the test suite is perhaps the major use case for this.

Yes, that could be a risk. Did you find it was happening during your POC? If so, could you please share some examples so that I can understand the circumstances better?

Yes. I can email examples if needed. Most common issues were:

• Improper decomposition of a tasks. I.e. having a list of Click, Enter, Click, Wait, Click... in a top level task.
• Pulling Page Object functionality into a private method of a task. Such as picking the input field based on task properties. Input fields themselves were however properly encapsulated in PO.
• Pulling too much of configuration of a task into static methods. So that those static methods might become tasks on their own.

The answers

make sure that there are no parameter name collisions? Several tasks could be easily using the same name for their parameter; how would defineSetters know which task we have in mind?

We can not prevent name collisions. As per current implementation all actions of the builder share all the props of a constructed task instance. They are supposed to be on the same abstraction level. So if a new action doesn't fit this abstraction level a composition should be used instead of amending an existing builder.

But an instantiated task doesn't receive any props from builder.

const EnterPersonalDetails = defineTask()
  .annotate('{0} born on #birthDate enters his personal details')
  .defineSetters({
    bornOn: 'birthDate'
  })
  .addActions(
    enterBirthDate,
    Click.on(PersonaDetailsForm.Submit)
  );

Click task in this example doesn't receive birthDate prop.

As props of a Task are part of its public interface if we are adding actions to an existing builder we should examine its interface and make sure we don't screw it. Or prefer composition where possible. In either case we would want to know the interface of the Task we are going to reuse.

There are however ways to improve this:

• We can throw a warning when defineSetters amends already existing props.
• Add a seal() method to explicitly finalize a builder and return Task instance to prevent reusing that builder.

make it obvious which task requires what parameter so that devs don't have to look for its source (encapsulation)?

Throw if not all prerequisites are provided?
For TS/flow I see two options: annotating an interface of a built task inline or providing a definition file. This should provide sufficient support by means of IDE.

how do we handle generating the annotation for the boolean fields, like priorityBoarding? We'd still need some sort of a method/function generating a string representation of the flag, would we not?

What about adding another method to supply serializers for props? Falling back to 'toString' if not provided. Something like defineTransforms(transforms: Map<propertyName, serializer>). This can also be used to define format for Dates or serializing complex objects.

defineTask()
  .defineSetters({
    withPriorityBoarding: 'priorityBoarding'      
  }),
  .defineTransforms({
    priorityBoarding: value => value ? 'PRIORITY' : 'REGULAR'      
  });

I will address the example with BookAReturnFlight tomorrow. Need to get some sleep.
Meanwhile I have these examples of handling name collision and using different ways of composition:

const enterName = (actor, {name}) => actor.atteptsTo(
  Enter.theValue(name)
    .into(PersonaDetailsForm.Name)
    .thenHit(Key.ENTER)
);

const enterBirthDate = (actor, {birthDate}) => actor.atteptsTo(
  Enter.theValue(birthDate)
    .into(PersonaDetailsForm.BirthDate)
    .thenHit(Key.ENTER)
);

const EnterPersonalDetails = defineTask()
  .annotate('{0} called #name born on #birthDate enters his personal details')
  .defineSetters({
    ofUser: 'name',
    bornOn: 'birthDate'
  })
  .addActions(
    enterName,
    enterBirthDate ,
    Click.on(PersonaDetailsForm.Submit)
  );

const enterCreditCard = (actor, {cardNumber}) => actor.atteptsTo(
  Enter.theValue(cardNumber)
    .into(CreditCardDetails.CardNumber)
    .thenHit(Key.ENTER)
);

const EnterCreditCardDetails = EnterPersonalDetails
  // This overrides annotation of EnterPersonalDetails
  .annotate('{0} called #name born on #birthDate posessing a card #cardNumber enters his payment details')
  // This adds new setter
  .defineSetters({
    posessingACard: 'cardNumber'
  })
  // This appends actions
  .addActions(
    enterCreditCard,
    Click.on(CreditCardDetailsForm.Submit)
  );
// All methods return a new instance of a builder and never mutate an existing one
// Therefore EnterPersonalDetails can still be used on its own after this.

const enterItemName = (actor, {name}) => actor.atteptsTo(
  Enter.theValue(name)
    .into(OrderForm.ItemName)
    .thenHit(Key.ENTER)
);

//Make action a pre-configured Task to compose
const enterCreditCardDetails = EnterPersonalDetails
  .ofUser('Joe')
  .bornOn('01.01.1970')
  .posessingACard('1234');

const CreateOrder = defineTask()
  .annotate('{0} orders item called #name')
  .defineSetters({
    forItem: 'name'
  })
  .addActions(
    enterItemName,
    Click.on(OrderForm.Submit) enterCreditCardDetails
  );

actor.atteptsTo(
  CreateOrder.forItem('Kitten')
);

Or expose payment details

const enterCreditCardDetails = (actor, {userName, birthDate, cardNumber}) => EnterPersonalDetails
  .ofUser(userName)
  .bornOn(birthDate)
  .posessingACard(cardNumber);

const CreateOrder = defineTask()
  .annotate('{0} orders item called #name')
  .defineSetters({
    forUser: 'userName',
    bornOn: 'birthDate'
    posessingACard: 'cardNumber',
    buying: 'name'
  })
  .addActions(
    Click.on(OrderForm.Submit) enterCreditCardDetails
  );

actor.atteptsTo(
  CreateOrder
    .forUser('Joe')
    .buying('Kitten')
    .bornOn('01.01.1970')
    .posessingACard('1234')
);

Or pass enterCreditCardDetails task as param

const orderItem = (actor, {name, enterCreditCardDetails}) => actor.atteptsTo(
  Enter.theValue(name)
    .into(OrderForm.ItemName)
    .thenHit(Key.ENTER),
  Click.on(OrderForm.Submit),
  enterCreditCardDetails
);

const CreateOrder = defineTask()
  .annotate('{0} orders item called #name')
  .defineSetters({
    forItem: 'name',
    providing: 'enterCreditCardDetails'
  })
  .addActions(
    orderItem
  );

const enterCreditCardDetails = EnterPersonalDetails
  .ofUser('Joe')
  .bornOn('01.01.1970')
  .posessingACard('1234');


actor.atteptsTo(
  CreateOrder
    .forItem('Kitten')
    .providing(enterCreditCardDetails)
);

[InvictusMB]

By the way I just realized that instead of relying on decorator for performAs for reporting SerenityJS should use a toString method defined for Task. toString could return the same templated string as we pass to step or any arbitrary text reflecting the state of a Task.
There would still be a need for a utility to plug notifications into performAs but that should be a separate concern anyway and it wouldn't necessary need a decorator.