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

Incremental rendering of HTML report #121

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aslakhellesoy
Copy link

🤔 What's changed?

Messages are written into the HTML report incrementally using a function call:

<script>p({...});</script>
<script>p({...});</script>
<script>p({...});</script>
...

⚡️ What's your motivation?

To render the partial report in a browser before the entire HTML has been written out. See #120 for details.

🏷️ What kind of change is this?

  • ⚡ New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

See README.md for an explanation of how to manually test incremental rendering.

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@aslakhellesoy
Copy link
Author

aslakhellesoy commented Jun 14, 2022

Left to do:

  • Replace fs/promises with util.promisify(fs.writeFile) to make tests pass on Node 12
  • Port changes in CucumberHtmlStream.ts to Java
  • Port changes in CucumberHtmlStream.ts to Ruby

@seleniumghost would you be able to help with any of these?

@seleniumghost
Copy link

I am trying to get it to work in Ruby.

@seleniumghost
Copy link

@aslakhellesoy

  • Port changes in CucumberHtmlStream.ts to Ruby

I think the corresponding file is \lib\cucumber\html_formatter.rb. I changed and attached it:
html_formatter.zip

But I could not get it to full work. I believe this is because in the Ruby version there exists a file \assets\cucumber-html.js which seems to need the changes from javascript/src/main.tsx. It's too big and complex to edit for me.

@mattwynne
Copy link
Member

I think this is a pretty neat solution. Intuitively it feels better to me to be processing these messages in one at a time rather than in a batch, anyway. I understand there are concerns about what happens if the message stream is in an unexpected order or incomplete, but it seems to me those scenarios would also cause problems with the current solution.

I don't think we should kid ourselves that we can turn this thing into a complete GUI for Cucumber, but to me this change seems worth the benefit as outlined in #120 (comment).

I would like to see some better acceptance tests - i.e. going end-to-end from an incomplete message stream to showing what the rendered page would look like. I've had a go at something in #124 but I think maybe something that asserts on specific expected elements in the rendered DOM would be better as you could actually TDD with that.

@aurelien-reeves
Copy link
Contributor

This would definitely prevent us to optimize the weight of the report by post processing the messages at the end of the run

@mattwynne
Copy link
Member

mattwynne commented Jun 16, 2022

This would definitely prevent us to optimize the weight of the report by post processing the messages at the end of the run

We definitely need to consider how this change will impact that problem (i.e. #62), which I think is more important to solve than the progressive rendering one. It would be a bad idea to make that one harder to solve.

I guess it depends on how we plan to achieve the post-processing. If we're going to process the messages outside of the HTML page, we could still use this mechanism to pass the processed message stream to the React rendering code on the page.

If the processor was written thoughtfully you might not even have to choose between progressive rendering and a lightweight HTML file: the processor could emit messages to the formatter as soon as it had received a coherent set of messages from the input.

@seleniumghost
Copy link

@aslakhellesoy or someone else who can answer this:
Is it true that \assets\cucumber-html.js in the Ruby version of Cucumber needs the changes from javascript/src/main.tsx? Is this built from another source?

@srbarrios
Copy link

Thank you very much for this draft. This is quite important for our test report reviews.

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.

5 participants