Support for attachments

[laughingbiscuit]

Looking through the docs, I cant see an attach dunction like in cucumber jvm/js/rb. The purpose would be to attach images or assets to the cucumber json result for display in a report.

Any thoughts?

https://github.com/cucumber/cucumber-js/blob/main/docs/support_files/attachments.md

https://javadoc.io/static/io.cucumber/cucumber-java8/6.9.1/io/cucumber/java8/Scenario.html#attach(byte%5B%5D,java.lang.String,java.lang.String)

[tyranron]

@laughingbiscuit thanks for proposal.

@ilslv would be nice to have this feature. Both .attach() and .log() (which finally mostly resolves our logging caveats).

It's also not obvious how images are rendered to console. Needs some further investigation.

[laughingbiscuit]

Thanks @tyranron

In my experience, images do not need to be rendered to the console itself, but instead their base64 representation stored in the cucumber json and then they are subsequently displayed in the HTML report only. Here is an example.

[ilslv]

@tyranron to be able to call .attach(...) we need something to call in on. And this is a great opportunity to redo some stuff about how we extract arguments in Step functions. I really like axum's extractors approach:

// From this
#[given("step")]
fn step(w: &mut World, step: Step) {}

// Or this
#[given("step")]
fn step(w: &mut World, #[step] s: Step) {}

// To this
#[given("step")]
fn step(w: &mut World, Context(step): Context<Step>) {}

This is extendable, future-proof solution, that even will allow us to add user provided context, if we want to.

---

Also specifying only FromStr arguments or &[String] seems to me a bit too limiting, why not allow both:

#[given(regex = "from (\\S+) to (\\S+)(?:(?:,| and) (\\S+))?(?:(?:,| and) (\\S+))?")]
fn step(w: &mut World, from: String, to: Vec<String>) {}
//              impl FromStr ^^^^^^      ^^^^^^^^^^^ impl FromIterator<Item = impl FromStr>

[tyranron]

@ilslv

Also specifying only FromStr arguments or &[String] seems to me a bit too limiting, why not allow both:

#[given(regex = "from (\\S+) to (\\S+)(?:(?:,| and) (\\S+))?(?:(?:,| and) (\\S+))?")]
fn step(w: &mut World, from: String, to: Vec<String>) {}
//              impl FromStr ^^^^^^      ^^^^^^^^^^^ impl FromIterator<Item = impl FromStr>

Yeah, this is definitely to have, but is a separate topic.

to be able to call .attach(...) we need something to call in on. And this is a great opportunity to redo some stuff about how we extract arguments in Step functions. I really like axum's extractors approach:

// From this
#[given("step")]
fn step(w: &mut World, step: Step) {}

// Or this
#[given("step")]
fn step(w: &mut World, #[step] s: Step) {}

// To this
#[given("step")]
fn step(w: &mut World, Context(step): Context<Step>) {}

Could you sketch how it should look with .attach() usage?

This is extendable, future-proof solution, that even will allow us to add user provided context, if we want to.

Actually, I don't understand, doesn't the World represent a "user provided context" here?

Maybe we could wire .attach() somehow to the World?

[ilslv]

@tyranron

Could you sketch how it should look with .attach() usage?

#[given("...")]
fn step(w: &mut World, cucumber::Extract(context): cucumber::Extract(cucumber::Context)) {
    context.attach(...)
}

Actually, I don't understand here, doesn't the World represent a "user provided context" here?

I meant global user-provided context. For now docs are saying that it's not responsibility of this crate, but this may be changed in the future.

Maybe we could wire .attach() somehow to the World?

Basically, we need a way to get event::Cucumber sender instance. This could be done with tracing-like contexts via thread locals, but it introduces problems with spawn(async {}), which I would like to avoid. So providing interface for extracting arbitrary types in steps is the only feasible thing to do in my opinion.

[ilslv]

Also we should provide cucumber::Context for before and after hooks

[tyranron]

@ilslv I'm still quite confused with the notion of cucumber::Context. We already have too many contexts here: World, Scenario, Step. What is cucumber::Context is about then?

If we look at JS implementation, it has .attach() method on World (usually called via this). Java has it directly on Scenario. Both feel rather straightforward and intuitive.

In our case, the Scenario is actually a gherkin::Scenario, so we cannot put such stuff here. Regarding this, I see the cucumber::Context being defined like this:

#[derive(Deref)]
struct Context<T> {
    #[deref]
    inner: T,
    // attachments and whatever
}

This way, we can add our cucumber context to existing gherkin elements:

#[given("step")]
fn step(w: &mut World, scenario: Context<Scenario>) {
    scenario.attach();
    scenario.steps; // accessible via `Deref`
}

Or maybe it's worth to attach the context to the World?

#[given("step")]
fn step(w: &mut Context<World>, scenario: Context<Scenario>) {
    w.attach();
}

Seem much more reasonable regarding &mut signature.

What do you think on that?

[ilslv]

@tyranron

What do you think on that?

I don't like the idea of making Context generic over World/Scenario/Step. I see it as a confusing way to hide complexity. cucumber::Context is a separate entity and has nothing to do with World or Scenario.

We already have too many contexts here: World, Scenario, Step. What is cucumber::Context is about then?

But I understand and partially agree with point made here. The solution I see is to make our contexts hierarchical: user-defined World and cucumber::Context are equally important, but Feature/Scenario/Rule/Step should be retrievable only from cucumber::Context as they are not really contexts in the same way.

We have 3 ways of implementing it:

1. World.context() method, that will use ThreadLocal variable to share Context. It's a non-breaking change and I like simplicity of it, but problem with loosing ThreadLocal context because of thread::spawn/tokio::spawn is unsolvable without special wrapper, which I would like to avoid.
2. Instead of #[step]/step: Step arguments, use _: Context (we can retrieve it using auto-deref specialization).
3. Support both 1 and 2.

---

All in all my proposal moving forward is following: replace #[step]/step: gherkin::Step with more general-purpose any_ident: cucumber::Context. This is a breaking change, but we can detect places where replacement is needed and emit understandable compile-time error.

[tyranron]

@ilslv if we make a cucumber::Context as a struct, consisting of gherkin::Step, gherkin::Scenario and some extra, seems quite reasonable. So, in places where we pass Scenario, Step and all the shitstuff, we can pass only the cucumber::Context, simplifying the signatures in many places. At the same time, if the user needs only Step we still can provide it to him directly, in a price of some simple under-the-hood extraction machinery on top of the cucumber::Context type.

Sounds reasonable. The only thing that feels strange here is the mutability question. As for now, Step/Scenario are read-only fields, and the only mutable context passed to the user is World which has the explicit &mut signature. If we put the mutable cucumber::Context into play, we should make either both via &mut, or both interior mutable. 🤔