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

Propagate Step being matched by multiple Regexes as AmbiguousMatchError #143

Merged
merged 30 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
3fd40d3
Add before and after hooks without propagating errors
ilslv Oct 14, 2021
72ae552
Implement hook events
ilslv Oct 15, 2021
b740341
Mention hooks in Features book chapter
ilslv Oct 15, 2021
c49280b
Corrections
ilslv Oct 15, 2021
192bace
CHANGELOG
ilslv Oct 15, 2021
3582127
WIP [skip ci]
ilslv Oct 18, 2021
eff31e5
Corrections
ilslv Oct 18, 2021
7b797bb
CHANGELOG, Docs and Book
ilslv Oct 18, 2021
e0075a8
Fix Windows tests
ilslv Oct 18, 2021
b484f8c
Fix Windows tests
ilslv Oct 18, 2021
2d16c86
Corrections
tyranron Oct 18, 2021
609c29d
Fix clap at beta.5 to prevent updating it with breaking changes
ilslv Oct 18, 2021
54b714e
Fix `clap` to `3.0.0-beta.5` version
tyranron Oct 18, 2021
558a9a4
Restore bad merge
tyranron Oct 18, 2021
0199ab6
Merge branch 'main' into return-before-and-after-hooks
tyranron Oct 18, 2021
23de704
Merge branch 'return-before-and-after-hooks' into ambiguous-error
ilslv Oct 18, 2021
c8919bd
Corrections
ilslv Oct 18, 2021
c6bf724
Merge branch 'main' into ambiguous-error
ilslv Oct 18, 2021
e88e14e
Corrections
ilslv Oct 18, 2021
2381dca
Merge branch 'main' into ambiguous-error
ilslv Oct 18, 2021
c8833f7
Corrections
ilslv Oct 18, 2021
5b7ecc2
Fmt
ilslv Oct 18, 2021
8ea254f
Merge branch 'main' into ambiguous-error
ilslv Oct 18, 2021
97d7b25
Some corrections [skip ci]
tyranron Oct 18, 2021
aee3645
Correction
ilslv Oct 19, 2021
a8b9a9e
Refactor Step::AmbiguousMatch into Step::Failed and StepError
ilslv Oct 19, 2021
3ff9036
Corrections
ilslv Oct 19, 2021
e54fa5b
Corrections
ilslv Oct 19, 2021
96db994
Corrections
ilslv Oct 19, 2021
837758e
Corrections
tyranron Oct 19, 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ All user visible changes to `cucumber` crate will be documented in this file. Th
- Made `#[step]` argument of step functions `Step` instead of `StepContext` again, while test callbacks still receive `StepContext` as a second parameter. ([#128])
- Deprecated `--nocapture` and `--debug` CLI options to be completely redesigned in `0.11` release. ([#137])
- [Hooks](https://cucumber.io/docs/cucumber/api/#hooks) now accept optional `&mut World` as their last parameter. ([#142])
- Propagate `Step` being matched by multiple `Regex`es as `AmbiguousMatchError` ([#143])

### Added

Expand All @@ -33,6 +34,7 @@ All user visible changes to `cucumber` crate will be documented in this file. Th
[#136]: /../../pull/136
[#137]: /../../pull/137
[#142]: /../../pull/142
[#142]: /../../pull/143



Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ async-trait = "0.1.40"
atty = "0.2.14"
clap = { version = "=3.0.0-beta.5", features = ["derive"] }
console = "0.14.1"
derive_more = { version = "0.99.16", features = ["deref", "display", "error", "from"], default_features = false }
derive_more = { version = "0.99.16", features = ["deref", "deref_mut", "display", "error", "from"], default_features = false }
either = "1.6"
futures = "0.3.17"
gherkin = { package = "gherkin_rust", version = "0.10" }
Expand Down
2 changes: 2 additions & 0 deletions book/src/Test_Modules_Organization.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ Avoid writing similar step definitions, as they can lead to clutter. While docum

## Managing growth

To help with non-intuitive and frustrating problems that can be encountered in case your `Step` is matched by multiple functions, annotated with `Step` attributes, we propagate that case as an [`AmbiguousMatchError`](https://docs.rs/cucumber/*/cucumber/event/enum.Step.html#variant.AmbiguousMatch).

As your test suit grows, it may become harder to notice how minimal changes to `regex`es can lead to mismatched `Step`s. To avoid this, we recommend using [`Cucumber::fail_on_skipped()`](https://docs.rs/cucumber/*/cucumber/struct.Cucumber.html#method.fail_on_skipped) combining with `@allow_skipped` tag. This will allow you to mark out `Scenario`s which `Step`s are allowed to skip.

And, as time goes on, total run time of all tests can become overwhelming when you only want to test small subset of `Scenario`s. At least until you discover [`Cucumber::filter_run_and_exit()`](https://docs.rs/cucumber/*/cucumber/struct.Cucumber.html#method.filter_run_and_exit), which will allow you run only `Scenario`s marked with custom [tags](https://cucumber.io/docs/cucumber/api/#tags).
Expand Down
125 changes: 70 additions & 55 deletions codegen/src/attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,73 @@ impl Step {

/// Expands generated code of this [`Step`] definition.
fn expand(self) -> syn::Result<TokenStream> {
let is_regex = matches!(self.attr_arg, AttributeArgument::Regex(_));

let func = &self.func;
let func_name = &func.sig.ident;

let (func_args, addon_parsing) = if is_regex {
let world = parse_world_from_args(&self.func.sig)?;
let constructor_method = self.constructor_method();
let (func_args, addon_parsing) =
self.fn_arguments_and_addition_parsing()?;

let step_matcher = self.attr_arg.regex_literal().value();
let caller_name =
format_ident!("__cucumber_{}_{}", self.attr_name, func_name);
let awaiting = if func.sig.asyncness.is_some() {
quote! { .await }
} else {
quote! {}
};
let step_caller = quote! {
{
#[automatically_derived]
fn #caller_name<'w>(
__cucumber_world: &'w mut #world,
__cucumber_ctx: ::cucumber::step::Context,
) -> ::cucumber::codegen::LocalBoxFuture<'w, ()> {
let f = async move {
#addon_parsing
#func_name(__cucumber_world, #func_args)#awaiting;
};
::std::boxed::Box::pin(f)
}

#caller_name
}
};

Ok(quote! {
#func

#[automatically_derived]
::cucumber::codegen::submit!(
#![crate = ::cucumber::codegen] {
<#world as ::cucumber::codegen::WorldInventory<
_, _, _,
>>::#constructor_method(
Some(::cucumber::step::Location {
path: ::std::convert::From::from(::std::file!()),
line: ::std::line!(),
column: ::std::column!(),
Copy link
Member

Choose a reason for hiding this comment

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

Given that we always provide cucumber::step::Location in the generated code, I think we may strip Option out of many places in of codegen machinery.

}),
::cucumber::codegen::Regex::new(#step_matcher)
.unwrap(),
#step_caller,
)
}
);
})
}

/// Generates code that prepares function's arguments based on
/// [`AttributeArgument`] and additional parsing if it's
/// [`AttributeArgument::Regex`].
fn fn_arguments_and_addition_parsing(
&self,
) -> syn::Result<(TokenStream, Option<TokenStream>)> {
let is_regex = matches!(self.attr_arg, AttributeArgument::Regex(_));
let func = &self.func;

if is_regex {
if let Some(elem_ty) = find_first_slice(&func.sig) {
let addon_parsing = Some(quote! {
let __cucumber_matches = __cucumber_ctx
Expand All @@ -131,7 +192,7 @@ impl Step {
.map(|arg| self.borrow_step_or_slice(arg))
.collect::<Result<TokenStream, _>>()?;

(func_args, addon_parsing)
Ok((func_args, addon_parsing))
} else {
#[allow(clippy::redundant_closure_for_method_calls)]
let (idents, parsings): (Vec<_>, Vec<_>) =
Expand All @@ -154,62 +215,16 @@ impl Step {
#( #idents, )*
};

(func_args, addon_parsing)
Ok((func_args, addon_parsing))
}
} else if self.step_arg_name.is_some() {
(
Ok((
quote! { ::std::borrow::Borrow::borrow(&__cucumber_ctx.step), },
None,
)
} else {
(TokenStream::default(), None)
};

let world = parse_world_from_args(&self.func.sig)?;
let constructor_method = self.constructor_method();

let step_matcher = self.attr_arg.regex_literal().value();
let caller_name =
format_ident!("__cucumber_{}_{}", self.attr_name, func_name);
let awaiting = if func.sig.asyncness.is_some() {
quote! { .await }
))
} else {
quote! {}
};
let step_caller = quote! {
{
#[automatically_derived]
fn #caller_name<'w>(
__cucumber_world: &'w mut #world,
__cucumber_ctx: ::cucumber::step::Context,
) -> ::cucumber::codegen::LocalBoxFuture<'w, ()> {
let f = async move {
#addon_parsing
#func_name(__cucumber_world, #func_args)#awaiting;
};
::std::boxed::Box::pin(f)
}

#caller_name
}
};

Ok(quote! {
#func

#[automatically_derived]
::cucumber::codegen::submit!(
#![crate = ::cucumber::codegen] {
<#world as ::cucumber::codegen::WorldInventory<
_, _, _,
>>::#constructor_method(
::cucumber::codegen::Regex::new(#step_matcher)
.unwrap(),
#step_caller,
)
}
);
})
Ok((TokenStream::default(), None))
}
}

/// Composes a name of the `cucumber::codegen::WorldInventory` method to
Expand Down
13 changes: 11 additions & 2 deletions codegen/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ fn generate_step_structs(
#[automatically_derived]
#[doc(hidden)]
#world_vis struct #ty {
#[doc(hidden)]
pub loc: ::std::option::Option<::cucumber::step::Location>,

#[doc(hidden)]
pub regex: ::cucumber::codegen::Regex,

Expand All @@ -72,17 +75,23 @@ fn generate_step_structs(
#[automatically_derived]
impl ::cucumber::codegen::StepConstructor<#world> for #ty {
fn new (
loc: ::std::option::Option<::cucumber::step::Location>,
regex: ::cucumber::codegen::Regex,
func: ::cucumber::Step<#world>,
) -> Self {
Self { regex, func }
Self { loc, regex, func }
}

fn inner(&self) -> (
::std::option::Option<::cucumber::step::Location>,
::cucumber::codegen::Regex,
::cucumber::Step<#world>,
) {
(self.regex.clone(), self.func.clone())
(
self.loc.clone(),
self.regex.clone(),
self.func.clone(),
)
}
}

Expand Down
40 changes: 26 additions & 14 deletions src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,18 @@ where
let mut out = step::Collection::new();

for given in Self::cucumber_given() {
let (regex, fun) = given.inner();
out = out.given(regex, fun);
let (loc, regex, fun) = given.inner();
out = out.given(loc, regex, fun);
}

for when in Self::cucumber_when() {
let (regex, fun) = when.inner();
out = out.when(regex, fun);
let (loc, regex, fun) = when.inner();
out = out.when(loc, regex, fun);
}

for then in Self::cucumber_then() {
let (regex, fun) = then.inner();
out = out.then(regex, fun);
let (loc, regex, fun) = then.inner();
out = out.then(loc, regex, fun);
}

out
Expand Down Expand Up @@ -143,8 +143,12 @@ where
///
/// [`given`]: crate::given
/// [Given]: https://cucumber.io/docs/gherkin/reference/#given
fn new_given(regex: Regex, fun: Step<Self>) -> G {
G::new(regex, fun)
fn new_given(
loc: Option<step::Location>,
regex: Regex,
fun: Step<Self>,
) -> G {
G::new(loc, regex, fun)
}

/// Returns an [`Iterator`] over items with [`when`] attribute.
Expand All @@ -159,8 +163,12 @@ where
///
/// [`when`]: crate::when
/// [When]: https://cucumber.io/docs/gherkin/reference/#when
fn new_when(regex: Regex, fun: Step<Self>) -> W {
W::new(regex, fun)
fn new_when(
loc: Option<step::Location>,
regex: Regex,
fun: Step<Self>,
) -> W {
W::new(loc, regex, fun)
}

/// Returns an [`Iterator`] over items with [`then`] attribute.
Expand All @@ -175,8 +183,12 @@ where
///
/// [`then`]: crate::then
/// [Then]: https://cucumber.io/docs/gherkin/reference/#then
fn new_then(regex: Regex, fun: Step<Self>) -> T {
T::new(regex, fun)
fn new_then(
loc: Option<step::Location>,
regex: Regex,
fun: Step<Self>,
) -> T {
T::new(loc, regex, fun)
}
}

Expand All @@ -190,8 +202,8 @@ where
pub trait StepConstructor<W> {
/// Creates a new [`Step`] with the corresponding [`Regex`].
#[must_use]
fn new(_: Regex, _: Step<W>) -> Self;
fn new(_: Option<step::Location>, _: Regex, _: Step<W>) -> Self;

/// Returns an inner [`Step`] with the corresponding [`Regex`].
fn inner(&self) -> (Regex, Step<W>);
fn inner(&self) -> (Option<step::Location>, Regex, Step<W>);
}
27 changes: 21 additions & 6 deletions src/cucumber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,26 +967,41 @@ impl<W, I, P, Wr, F, B, A> Cucumber<W, P, I, runner::Basic<W, F, B, A>, Wr> {
///
/// [Given]: https://cucumber.io/docs/gherkin/reference/#given
#[must_use]
pub fn given(mut self, regex: Regex, step: Step<W>) -> Self {
self.runner = self.runner.given(regex, step);
pub fn given(
mut self,
loc: Option<step::Location>,
regex: Regex,
step: Step<W>,
) -> Self {
self.runner = self.runner.given(loc, regex, step);
self
}

/// Inserts [When] [`Step`].
///
/// [When]: https://cucumber.io/docs/gherkin/reference/#When
#[must_use]
pub fn when(mut self, regex: Regex, step: Step<W>) -> Self {
self.runner = self.runner.when(regex, step);
pub fn when(
mut self,
loc: Option<step::Location>,
regex: Regex,
step: Step<W>,
) -> Self {
self.runner = self.runner.when(loc, regex, step);
self
}

/// Inserts [Then] [`Step`].
///
/// [Then]: https://cucumber.io/docs/gherkin/reference/#then
#[must_use]
pub fn then(mut self, regex: Regex, step: Step<W>) -> Self {
self.runner = self.runner.then(regex, step);
pub fn then(
mut self,
loc: Option<step::Location>,
regex: Regex,
step: Step<W>,
) -> Self {
self.runner = self.runner.then(loc, regex, step);
self
}
}
Expand Down
Loading