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

Wrap event::Cucumber in Event to allow metadata #145

Merged
merged 29 commits into from
Oct 26, 2021
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f031fc2
Add --verbose flag and remove legacy CLI options
ilslv Oct 20, 2021
a4a8f62
Add --scenario-name and --scenario-tags CLI options
ilslv Oct 20, 2021
f0220bb
Add --features option
ilslv Oct 20, 2021
696de67
Correction
ilslv Oct 20, 2021
7b24407
Add Cucumber methods for additional user CLI
ilslv Oct 20, 2021
d9c2589
Correction
ilslv Oct 21, 2021
ba2eccf
Merge branch 'main' into 134-cli-redesign
ilslv Oct 21, 2021
0b58059
Corrections
ilslv Oct 21, 2021
d7bb724
Corrections
ilslv Oct 21, 2021
c48d942
Corrections and add todo_or_die crate
ilslv Oct 21, 2021
4119545
Corrections
ilslv Oct 21, 2021
f0c65db
Lint
ilslv Oct 21, 2021
7c47f3b
Try removing todo-or-die crate
ilslv Oct 21, 2021
90cab88
Add Custom generic parameter to cli::Opts
ilslv Oct 21, 2021
2e83a94
Mention CLI in Features chapter of the book
ilslv Oct 21, 2021
d104147
Bump version to 0.11 and add CHANGELOG entry
ilslv Oct 21, 2021
75f542a
Some corrections [skip ci]
tyranron Oct 21, 2021
cf8e669
Corrections
ilslv Oct 22, 2021
d345c4c
Move trait bound from impls to struct in Cucumber
ilslv Oct 22, 2021
c2d254c
Corrections
ilslv Oct 22, 2021
ff891a9
Pair DateTime with each event
ilslv Oct 22, 2021
598d0bc
Correction
ilslv Oct 22, 2021
be20764
Fix chrono at 0.4.19 to avoid problems with minimal-versions
ilslv Oct 25, 2021
57ed9ea
Merge branch 'main' into add-datetime-to-events
tyranron Oct 25, 2021
aab2e27
Refactor `(Event, DateTime<Utc>)` into `DateTimed<Event>`
ilslv Oct 26, 2021
0f4d2ea
Wrap `event::Cucumber` in `Event<T>`
ilslv Oct 26, 2021
7808d76
Correction
ilslv Oct 26, 2021
39ba0df
Corrections [run ci]
tyranron Oct 26, 2021
3c4e144
Add changelog entry
tyranron Oct 26, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ macros = ["cucumber-codegen", "inventory"]
[dependencies]
async-trait = "0.1.40"
atty = "0.2.14"
chrono = "0.4.19"
console = "0.15"
derive_more = { version = "0.99.16", features = ["deref", "deref_mut", "display", "error", "from"], default_features = false }
either = "1.6"
Expand Down
4 changes: 2 additions & 2 deletions src/cucumber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,8 +882,8 @@ where

let events_stream = runner.run(filtered, runner_cli);
futures::pin_mut!(events_stream);
while let Some(ev) = events_stream.next().await {
writer.handle_event(ev, &writer_cli).await;
while let Some((ev, at)) = events_stream.next().await {
writer.handle_event(ev, at, &writer_cli).await;
}
writer
}
Expand Down
33 changes: 24 additions & 9 deletions src/runner/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use std::{
},
};

use chrono::{DateTime, Utc};
use futures::{
channel::mpsc,
future::{self, Either, LocalBoxFuture},
Expand Down Expand Up @@ -386,8 +387,10 @@ where
{
type Cli = Cli;

type EventStream =
LocalBoxStream<'static, parser::Result<event::Cucumber<W>>>;
type EventStream = LocalBoxStream<
'static,
(parser::Result<event::Cucumber<W>>, DateTime<Utc>),
Copy link
Member

@tyranron tyranron Oct 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, this is not very good decision, now we have the code base polluted with those tuples and signatures became worse.

I see here two ways:

  1. Attach DateTime directly to events types and errors.
  2. Instead of (T, DateTime) pairs have DateTimed<T> wrapper, which encapsulates the stuff and provides obvious interface to inner values.

With the option 2, we will need to only change type signatures, without touching any code. Furthermore, this way we can hide it completely behind a feature gate, so it can be used only for those Writers which really need it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tyranron

Attach DateTime directly to events types and errors.

As all events are nested enums, we'll have to put DateTime in every variant of the Cucumber event

pub enum Cucumber<World> {
    /// [`Cucumber`] execution being started.
    Started(DateTime<Utc>),

    /// [`Feature`] event.
    Feature(DateTime<Utc>, Arc<gherkin::Feature>, Feature<World>),

    /// [`Cucumber`] execution being finished.
    Finished(DateTime<Utc>),
}

Matching over it will be very painful.

Instead of (T, DateTime) pairs have DateTimed wrapper, which encapsulates the stuff and provides obvious interface to inner values.

👍

With the option 2, we will need to only change type signatures, without touching any code.

Not really, especially in case of writer::Normalized. There we store only event::Step and path to it: Feature, Option<Rule>, Scenario. Then we rebuild it with Cucumber::scenario(feat, rule, sc, ev) when it's time to emit it. But it's pretty easy to store DateTimes of those events and already done in this PR.

Furthermore, this way we can hide it completely behind a feature gate, so it can be used only for those Writers which really need it.

I'm not sure we should. This will cause Writer trait to be different in cases when feature is on or off (same for Runner)

pub trait Writer<World> {
    type Cli: StructOptInternal;

    async fn handle_event(
        &mut self,
        ev: parser::Result<event::Cucumber<World>>,
        #[cfg(feature = "event_datetime")] // I don't think it's pleasant API to use
        at: DateTime<Utc>,
        cli: &Self::Cli,
    );
}

>;

fn run<S>(self, features: S, cli: Cli) -> Self::EventStream
where
Expand Down Expand Up @@ -442,7 +445,10 @@ async fn insert_features<W, S, F>(
into: Features,
features: S,
which_scenario: F,
sender: mpsc::UnboundedSender<parser::Result<event::Cucumber<W>>>,
sender: mpsc::UnboundedSender<(
parser::Result<event::Cucumber<W>>,
DateTime<Utc>,
)>,
) where
S: Stream<Item = parser::Result<gherkin::Feature>> + 'static,
F: Fn(
Expand All @@ -459,7 +465,7 @@ async fn insert_features<W, S, F>(
// If the receiver end is dropped, then no one listens for events
// so we can just stop from here.
Err(e) => {
if sender.unbounded_send(Err(e)).is_err() {
if sender.unbounded_send((Err(e), Utc::now())).is_err() {
break;
}
}
Expand All @@ -476,7 +482,10 @@ async fn execute<W, Before, After>(
features: Features,
max_concurrent_scenarios: Option<usize>,
collection: step::Collection<W>,
sender: mpsc::UnboundedSender<parser::Result<event::Cucumber<W>>>,
sender: mpsc::UnboundedSender<(
parser::Result<event::Cucumber<W>>,
DateTime<Utc>,
)>,
before_hook: Option<Before>,
after_hook: Option<After>,
) where
Expand Down Expand Up @@ -584,7 +593,10 @@ struct Executor<W, Before, After> {
/// Sender for notifying state of [`Feature`]s completion.
///
/// [`Feature`]: gherkin::Feature
sender: mpsc::UnboundedSender<parser::Result<event::Cucumber<W>>>,
sender: mpsc::UnboundedSender<(
parser::Result<event::Cucumber<W>>,
DateTime<Utc>,
)>,
}

impl<W: World, Before, After> Executor<W, Before, After>
Expand All @@ -609,7 +621,10 @@ where
collection: step::Collection<W>,
before_hook: Option<Before>,
after_hook: Option<After>,
sender: mpsc::UnboundedSender<parser::Result<event::Cucumber<W>>>,
sender: mpsc::UnboundedSender<(
parser::Result<event::Cucumber<W>>,
DateTime<Utc>,
)>,
) -> Self {
Self {
features_scenarios_count: HashMap::new(),
Expand Down Expand Up @@ -1098,7 +1113,7 @@ where
fn send(&self, event: event::Cucumber<W>) {
// If the receiver end is dropped, then no one listens for events
// so we can just ignore it.
drop(self.sender.unbounded_send(Ok(event)));
drop(self.sender.unbounded_send((Ok(event), Utc::now())));
}

/// Notifies with the given [`Cucumber`] events.
Expand All @@ -1108,7 +1123,7 @@ where
for ev in events {
// If the receiver end is dropped, then no one listens for events
// so we can just stop from here.
if self.sender.unbounded_send(Ok(ev)).is_err() {
if self.sender.unbounded_send((Ok(ev), Utc::now())).is_err() {
break;
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/runner/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

pub mod basic;

use chrono::{DateTime, Utc};
use futures::Stream;
use structopt::StructOptInternal;

Expand Down Expand Up @@ -75,8 +76,10 @@ pub trait Runner<World> {
// details being a subject of instability.
type Cli: StructOptInternal;

/// Output events [`Stream`].
type EventStream: Stream<Item = parser::Result<event::Cucumber<World>>>;
/// Output events [`Stream`] paired with a [`DateTime`] when they happened.
type EventStream: Stream<
Item = (parser::Result<event::Cucumber<World>>, DateTime<Utc>),
>;

/// Executes the given [`Stream`] of [`Feature`]s transforming it into
/// a [`Stream`] of executed [`Cucumber`] events.
Expand Down
2 changes: 2 additions & 0 deletions src/writer/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::{
};

use async_trait::async_trait;
use chrono::{DateTime, Utc};
use console::Term;
use itertools::Itertools as _;
use regex::CaptureLocations;
Expand Down Expand Up @@ -106,6 +107,7 @@ impl<W: World + Debug> Writer<W> for Basic {
async fn handle_event(
&mut self,
ev: parser::Result<event::Cucumber<W>>,
_: DateTime<Utc>,
cli: &Self::Cli,
) {
use event::{Cucumber, Feature};
Expand Down
4 changes: 3 additions & 1 deletion src/writer/fail_on_skipped.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use std::sync::Arc;

use async_trait::async_trait;
use chrono::{DateTime, Utc};
use derive_more::Deref;

use crate::{event, parser, ArbitraryWriter, FailureWriter, World, Writer};
Expand Down Expand Up @@ -64,6 +65,7 @@ where
async fn handle_event(
&mut self,
ev: parser::Result<event::Cucumber<W>>,
at: DateTime<Utc>,
cli: &Self::Cli,
) {
use event::{
Expand Down Expand Up @@ -95,7 +97,7 @@ where
_ => ev,
};

self.writer.handle_event(ev, cli).await;
self.writer.handle_event(ev, at, cli).await;
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/writer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub mod summarized;
pub mod term;

use async_trait::async_trait;
use chrono::{DateTime, Utc};
use sealed::sealed;
use structopt::StructOptInternal;

Expand Down Expand Up @@ -63,6 +64,7 @@ pub trait Writer<World> {
async fn handle_event(
&mut self,
ev: parser::Result<event::Cucumber<World>>,
at: DateTime<Utc>,
cli: &Self::Cli,
);
}
Expand Down
Loading