Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename again #31

Closed
aslakhellesoy opened this issue Jul 7, 2021 · 23 comments
Closed

Rename again #31

aslakhellesoy opened this issue Jul 7, 2021 · 23 comments

Comments

@aslakhellesoy
Copy link
Contributor

The current new name is bound to create the false impression that this integrates with https://github.com/microsoft/playwright

I’ll rename to dollshouse next week as I suggested in #25 - unless someone convinces me of a better name first.

/cc @mattwynne @vincent-psarga

@mattwynne
Copy link
Member

mattwynne commented Jul 7, 2021

Ugh. Shame. I really like "playwright".

Why dollshouse? What's the metaphor?

Other ideas:

  • screenplay-lite
  • action-stations
  • ...

Honestly, I disagree with @jan-molak's assertion in #25 that this isn't faithful enough to the screenplay pattern to share the same name. I think it's close enough to be relevant and easily found from a search query. The differences are interesting and perhaps just reflect an evolution of the pattern. Certainly I think what we've done here with polymorphism for interactions is really important work.

Also, I think we should add Tasks to this library, because they're awesome.

I'd be interested in @wakaleo / @antonymarcano / @andypalmer's takes on this.

@jan-molak
Copy link
Member

jan-molak commented Jul 7, 2021

Hey @mattwynne - it feels like the main focus of this library is to build on and expand on @aslakhellesoy's earlier work on sub-second TDD and make multi-layer testing easier thanks to polymorphism. I agree with you that this work is important and valuable.

However, I still stand by my opinion that this pattern is significantly different from the Screenplay Pattern, which has five components (Actors, Abilities, Interactions, Tasks and Questions, all serving a specific purpose), and is primarily focused on modelling multi-actor workflows.

In this implementation, there are no abilities, no tasks, and questions and interactions are interchangeable. So I don't think that presenting it as an "evolution" of the Screenplay Pattern is accurate; it is a different pattern and deserves its own name.

Furthermore, I'd argue that by dropping abilities as proposed here, actors become a simple representation of state. This means that you could actually get away without actors altogether and simply pass Cucumber World object to action functions instead. Or, set a world property on this in action functions. This approach does also feel like a more idiomatic Cucumber way.

So, if we've removed tasks, questions, abilities, and actors from the Screenplay Pattern, all that we'd be left with are interactions and a polymorphic loading mechanism (which is awesome, by the way).

Now, if we added tasks back into the mix, as per your suggestion above, we'd arrive at one of @antonymarcano's earlier models that preceded the Screenplay Pattern - see Goals, Tasks, Actions.

Since this feels more in line with the direction of this library, perhaps it would make sense to highlight its focus on actions, and making them polymorphic, and call it say @cucumber/actions?


I was thinking about it some more; I think that by using only tasks and interactions, the example from the readme could be reduced even further.

From:

When('{actor} logs in', async function (actor: Actor) {
  // The logIn() function is an Interaction
  await actor.attemptsTo(logIn(`${actor.name}@test.com`, 'valid-password'))
})

to:

When('{name} logs in', name => logIn(`${name}@test.com`, 'valid-password'))

where:

export const logIn: LogIn = (email, password) => {
  return async function (this: { world: World }) {
    this.world.shouty // etc
  }
}

This approach would give Cucumber a way to load step definitions the same you'd load our hypothetical "Cucumber action", which I think is an interesting area to explore.

@aslakhellesoy
Copy link
Contributor Author

Why dollshouse? What's the metaphor?

It's a play by playwright Henrik Ibsen and one of the most played plays ever. It is also regarded as a great contribution to feminist literature.

For people who don't know the play, I think "doll's house" would invoke associations of multiple actors (that's what you typically find in kids' dolls houses). We could emphasise this with a logo picturing multiple characters in a house (or a cage, as has been used for theatre posters).

And most importantly - I think libraries with original names are more fun - "screenplay" is a bit dull IMHO :-)

Honestly, I disagree with @jan-molak's assertion in #25 that this isn't faithful enough to the screenplay pattern to share the same name. I think it's close enough to be relevant and easily found from a search query.

Ok, let's revisit that discussion.

Abilities

I said previously that playwright doesn't implement them. What I meant is that it doesn't implement them as an actor has abilities OO relationship. However, you could argue that the polymorphic interactions described here and implemented here is just a different implementation of abilities that achieve the same goal. The main difference is that deciding what abilities are available is done in configuration (world parameters) rather than in code.

Tasks

In fact, I think what we currently call interactions in this library should be renamed to tasks. I also think that the decomposition of tasks into interactions is something the library doesn't need an API for. It's just delegation/extract method, and could be handled by user code.

Questions

They are supported through the Actor#ask method.

Also, I think we should add Tasks to this library, because they're awesome.

See my comment above. Screenplay is a pattern, and it is not prescriptive of what parts of the pattern should be implemented in a library, and what should be implemented in user code. I believe decomposing tasks into interactions can be handled by user code and still be faithful to the pattern.

@wakaleo
Copy link
Contributor

wakaleo commented Jul 8, 2021

I would say that the actors, tasks, questions and abilities are pretty central to the DNA of the Screenplay pattern. What you are describing sounds more like what I called Action Classes in https://johnfergusonsmart.com/page-objects-that-suck-less-tips-for-writing-more-maintainable-page-objects/, and what Cypress refer to as "App Actions". I agree with Jan in that, which there are similarities with the Screenplay pattern, without the activities and questions it really feels like its own thing.

A rendering library could be used as part of an implementation of an MVC pattern, but in itself it does not implement the pattern. The other Screenplay implementations out there, such as the .NET (https://automationpanda.com/2020/10/16/introducing-boa-constrictor-the-net-screenplay-pattern/) and Python (https://screenpy-docs.readthedocs.io/en/latest/) respect the Actors/Abilities/Tasks/Interactions/Questions architecture pretty rigorously.

@antonymarcano
Copy link

antonymarcano commented Jul 9, 2021

I don't agree that the fully-fledged implementation is entirely necessary in all cases.
I would say, in its purest state, it's about Actors and Tasks... Here, when you read Task, think the what – e.g. "Make a call". When you read Actions, think the how – e.g. "unlock phone, open contacts, enter 'Aslak' into the search box... etc).

Everything else is just options. Actions can just be imperative lines of code doing things. Questions were just a way to have fluent assertions using a Given/When/Then style in code and due to the constraints of Java largely. I'm not sure that's as relevant with Cucumber (or javascript).

If I need to have a return type for a Task, that would be an Outcome – something you can use to return state (or not).

When I relate this to Cucumber, I explain it something like this:
Goals: the Scenario title is written as the user's goal (e.g. Scenario: Get feedback on why my email was invalid)
-> Tasks: Steps say 'what' the actor is doing (and the Step Def acts as a thin adapter delegating to a Task)
--> Actions: are the 'how' concrete things an actor would do – think 'interactions' – one or more of which will be inside the body of a Task. This could just be imperative lines of code, or that too can be abstracted behind the concept of an Action... Hence, the Actor and Task is the more important concept.

The pattern was intended to break the structural thinking binding people to a 'page' concept (which continued long after the concept of a page in web apps became irrelevant) and to do the same for APIs – enabling what I demonstrated in 2012the code for which is here.

Instead, I wanted to encourage people to model the domain – that there are people using our products to complete tasks (like "Pay by Card") to achieve a goal (like "Buy a Book"). The benefits of the resulting abstractions will be clear to this group.

I encourage you to consider thinking of the Actor performing Tasks... and to feel free to continue the exploration of the ideas inspired by Screenplay – under the same name if you would like to... Or under another name if you prefer.

@aurelien-reeves
Copy link
Contributor

aurelien-reeves commented Jul 12, 2021

I think @cucumber/dollshouse is fine 👍 .

I have also the following suggestion: @cucumber/scene. As the scene is where actors play their parts
Or @cucumber/theater

In those cases, each test domain could be referred as theater sets, backdrops, or scenaries.

@mattwynne
Copy link
Member

I like those suggestions @aurelien-reeves. @cucumber/stageplay?

@mattwynne
Copy link
Member

But, having now read @antonymarcano's reply I suggest we rename what we've called "interactions" to "tasks", and then keep the name screenplay. I think I'd misunderstood the distinction between those two a little bit.

I can imagine that this library might evolve to support these other concepts as the need for them becomes clearer.

I'm super curious about how we can fit the polymorphism concept into the screenplay metaphor. I'd personally called these Perspectives in a previous iteration of this idea, my thinking being that Actors carry out their Actions through a Perspective.

I have to admit I never really understood Abilities, so not sure where they would fit into this idea.

@antonymarcano
Copy link

antonymarcano commented Jul 12, 2021 via email

@aslakhellesoy
Copy link
Contributor Author

But, having now read @antonymarcano's reply I suggest we rename what we've called "interactions" to "tasks", and then keep the name screenplay. I think I'd misunderstood the distinction between those two a little bit.

I agree.

Decomposing a task into multiple interactions is something I don't think the library needs to provide an API for. It's just an extract method refactoring. It's a concept of the pattern that doesn't need to be implemented in a library. The programming language provides the constructs you need.

@aslakhellesoy
Copy link
Contributor Author

I've addressed the renames in #34

Here is an example of a task that is decomposed into interactions.

@aslakhellesoy
Copy link
Contributor Author

Now that #34 illustrates the following:

  • Actors
  • Tasks
  • Questions
  • Interactions (aka Actions)
  • Abilities (implemented as dynamic loading of polymorphic task implementations)

...do we have your blessing to rename this back to @cucumber/screenplay @antonymarcano?

(BTW, I have updated the credits section with more historical links dating back to 2007).

@aslakhellesoy
Copy link
Contributor Author

🥺 @antonymarcano 🥺

@aslakhellesoy
Copy link
Contributor Author

@mattwynne I think we should rename it back to @cucumber/screenplay.js since Antony hasn't objected. The playwright name is already causing confusion:

I'm on holiday for another few weeks - do you have time to do this in the meanwhile?

@mattwynne
Copy link
Member

Done! Bon vaccances @aslakhellesoy! 🏖️

@aslakhellesoy
Copy link
Contributor Author

aslakhellesoy commented Jul 29, 2021

Thanks 😊

I’m reopening this since it’s not done yet. Left to do:

  • Rename module in package.json
  • Undeprecate the deprecated @cucumber/screenplay module on npm
  • Deprecate @cucumber/playwright on npm
  • Rename any mentions in code and docs
  • Make a new release as @cucumber/screenplay

@aslakhellesoy aslakhellesoy reopened this Jul 29, 2021
@aurelien-reeves
Copy link
Contributor

aurelien-reeves commented Jul 29, 2021

Just noticed that I am still not part of the @cucumber organization on npmjs.com.
Could I be invited? That way I could make some progress with that issue

Also, how do we deprecate things on NPM? Is it just some mentions in the README? If so we will have to release a new version of playwright which deprecated it before anything else.

@aslakhellesoy
Copy link
Contributor Author

Just noticed that I am still not part of the @cucumber organization on npmjs.com.

You don't need to be. Use the cukebot account - credentials are in the keybase secret repo.

Also, how do we deprecate things on NPM?

Google it :-) https://docs.npmjs.com/deprecating-and-undeprecating-packages-or-package-versions

@aurelien-reeves
Copy link
Contributor

Just noticed that I am still not part of the @cucumber organization on npmjs.com.

You don't need to be. Use the cukebot account - credentials are in the keybase secret repo.

👍 thanks

Also, how do we deprecate things on NPM?

Google it :-) https://docs.npmjs.com/deprecating-and-undeprecating-packages-or-package-versions

😶 Yes, of course. Sorry :(

@aurelien-reeves
Copy link
Contributor

aurelien-reeves commented Jul 30, 2021

"Rename any mentions in code and docs"

Is there any other doc than the README?

Beside that, everything is done.

I had a hard time undeprecating the @cucumber/screenplay because of an issue with NPM, you may have received a few emails because of that 😅

If you are curious: the issue I had with NPM has already been fixed, but not been released yet: npm/cli#3484

@aslakhellesoy
Copy link
Contributor Author

Great! Thanks for doing this. The README.md is the only docs btw

@aslakhellesoy
Copy link
Contributor Author

There is an open PR - would be good to get that merged before doing a release.

@aurelien-reeves
Copy link
Contributor

There is an open PR - would be good to get that merged before doing a release.

Too late :(

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

No branches or pull requests

6 participants