-
Notifications
You must be signed in to change notification settings - Fork 71
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
Restore before and after hooks (#141) #142
Conversation
FCM
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ilslv good, but please tend to do more polishing in future. There were plenty of little inconveniences.
src/event.rs
Outdated
/// [`Scenario`]: gherkin::Scenario | ||
/// [`Step`]: gherkin::Step | ||
#[derive(Clone, Copy, Debug)] | ||
pub enum HookTy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In type names it's better not to use shortcuts. They're good for local variables only.
src/event.rs
Outdated
Failed(Option<Arc<World>>, Info), | ||
} | ||
|
||
impl fmt::Display for HookTy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, do not mix declarations and impl
s of different types. It's very misleading.
src/event.rs
Outdated
HookTy::Before => "Before", | ||
HookTy::After => "After", | ||
}; | ||
write!(f, "{}", s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just piggyback to Debug
impl here.
tests/wait.rs
Outdated
|
||
let err = AssertUnwindSafe(res) | ||
.catch_unwind() | ||
.await | ||
.expect_err("should err"); | ||
let err = err.downcast_ref::<String>().unwrap(); | ||
|
||
assert_eq!(err, "2 steps failed, 1 parsing error"); | ||
assert_eq!(err, "2 steps failed, 1 parsing error, 0 hook errors"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, it has little sense to write something like "0 errors has happened". We should write about errors only when they really happen.
@@ -209,6 +219,33 @@ impl Basic { | |||
} | |||
} | |||
|
|||
fn hook_failed<W: Debug>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docs 😱 ???
0199ab6
to
3d30bee
Compare
Resolves #141
Synopsis
In
0.10.0
before and after hooks were removed.Solution
Re-implement them.
Checklist
Draft:
prefixDraft:
prefix is removedThis PR is nominated at RU RustCon Contest