-
Notifications
You must be signed in to change notification settings - Fork 12
Render a more test case-centric report #396
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
Conversation
✅ Deploy Preview for cucumber-react-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
6531899 to
ece2089
Compare
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.
Not a strictly necessary refactor, but it was helpful to separate the accordion rendering from the wrangling of Gherkin documents in <GherkinDocumentList/>
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.
Copied from @cucumber/query - we should consider exposing it there as a plain function or static method.
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.
Some search tests were disabled but actually pass - they may have been fixed back in #337
src/components/gherkin/MDG.tsx
Outdated
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.
An alternate render tree for Markdown, given its pretty low adoption, is not worth the cost at this point. I think we'd be better off making the Markdown parser include extra content in the descriptions.
I think it would make sense to create an issue for this and then see what interest there is. While they're not recommend, backgrounds exist long enough that someone is probably using them. |
I hadn't seen your comment, but today I've pushed a change where we show just the name and description if either is populated, but not the steps. That way if there is meaningful text in a Background it doesn't get lost, but the steps are still firmly within the test cases. Let's see what the feedback is on this. |
🤔 What's changed?
Change the way documents are rendered to be more test case centric. The main effect is:
A good illustration is with the examples tables sample:
Before
After
⚡️ What's your motivation?
Part of cucumber/html-formatter#283
Fixes #389
This isn't the big overhaul I thought it would be - taking the suggestion to mostly keep the Gherkin hierarchy and just shift the results rendering made this much more a cleaning up and slimming down of the current thing. It's a breaking change only because of the custom rendering - many of the components that could be overridden before are gone now, but otherwise the customisation on Gherkin elements like doc strings, tables, attachments, tags etc is the same as before.
🏷️ What kind of change is this?
♻️ Anything particular you want feedback on?
Backgrounds are not shown separately now as all their steps are inlined in the test cases. But there's a potential for loss of useful info if e.g. a Background includes a title and/or description. We could look to still include it in some way, maybe conditionally if there is non-empty content, but even that feels low reward to me, especially given Backgrounds are basically not recommended these days.
The search works the same way as before. There's definitely a good opportunity to rework that to be simpler and more efficient now, but this PR is already big enough so I'll take care of that in the next one.
📋 Checklist:
This text was originally generated from a template, then edited by hand. You can modify the template here.