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

feat: also pass the name of the things to hooks #55

Merged
merged 6 commits into from Nov 3, 2020
Merged

feat: also pass the name of the things to hooks #55

merged 6 commits into from Nov 3, 2020

Conversation

maraisr
Copy link
Sponsor Contributor

@maraisr maraisr commented Nov 2, 2020

This PR aims to also pass through the name of either the suite or the test through to hooks.

This would aid in things like playwright and taking a screenshot after every test, and naming it the same as the test.

@codecov-io
Copy link

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is 80.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #55   +/-   ##
=======================================
  Coverage   90.16%   90.16%           
=======================================
  Files           3        3           
  Lines         305      305           
=======================================
  Hits          275      275           
  Misses         30       30           
Impacted Files Coverage Δ
src/index.js 78.09% <80.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30c10e0...307a826. Read the comment docs.

src/index.js Outdated Show resolved Hide resolved
@lukeed
Copy link
Owner

lukeed commented Nov 2, 2020

Hmm.. what about a string[] value instead? Eg, ['suite name', 'single test name'] – a list of "breadcrumbs" could be more flexible down the road, since just passing around a string now would require a bit of parsing to get back to that suite-vs-test information. And it'd be flaky since it's not as simple as name.split(' ')

Another option could be writing into context.__test__ (name tbd) with the same information.

@maraisr
Copy link
Sponsor Contributor Author

maraisr commented Nov 2, 2020

Thanks for that @lukeed i think i've done it, and kinda "Lukey" with that crumbs array thingo. Let me know what you feel.

src/index.d.ts Outdated Show resolved Hide resolved
Copy link
Owner

@lukeed lukeed left a comment

Choose a reason for hiding this comment

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

Going to be applying some light renames

src/index.d.ts Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
Copy link
Owner

@lukeed lukeed left a comment

Choose a reason for hiding this comment

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

Ok. We discussed this a little bit & agreed that using special context keys was the better way to go. It allows hooks and test handlers to maintain the same API & doesn't litter the Callback signature with extra, conditional parameters.

@maraisr Would appreciate it if you were able to look over the changes & verify that they still match your expectations. Thanks!

src/index.d.ts Show resolved Hide resolved
src/index.js Show resolved Hide resolved
src/index.js Show resolved Hide resolved
@maraisr
Copy link
Sponsor Contributor Author

maraisr commented Nov 3, 2020

ship it! 🚀

@lukeed lukeed merged commit 28d12e9 into lukeed:master Nov 3, 2020
@maraisr maraisr deleted the adds-name-hooks branch November 3, 2020 22:48
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

Successfully merging this pull request may close these issues.

None yet

3 participants