serenity-bdd / serenity-core

Serenity BDD is a test automation library designed to make writing automated acceptance tests easier, and more fun.
http://serenity-bdd.info
Other
721 stars 517 forks source link

Scenario Outlines with example params in title break report formatting #3522

Open eitzenbe opened 2 months ago

eitzenbe commented 2 months ago

What happened?

Cloned serenity-cucumber-starter project, upgraded to serenity version 4.1.20 (but also fails with original version)

changed feature file to:

Feature: Search by keyword

@green Scenario: Searching for 'green' Given Sergey is researching things on the internet When he looks up "green" Then he should see information about "green"

@red Scenario Outline: Searching for "" Given Sergey is researching things on the internet When he looks up "" Then he should see information about "" Examples: |color| |blue |

Report looks strange:

image

First entry is from a version where the title was without "<color" but only in title

image

Side Note: This happens only on testresults pages, on the feature pages its all fine:

image

image

What did you expect to happen?

The Testresults pages/tables show the title with correctly escaped "<" and ">" tags

Serenity BDD version

4.1.20

JDK version

jdk17

Execution environment

Linux

How to reproduce the bug.

see above, minimal changes to be applied to serenity-cucumber-starter project

How can we make it happen?

Work on this myself and propose a PR (with Serenity BDD team guidance)

wakaleo commented 2 months ago

Well spotted - the thing to do would be to check the reporting logic for the feature files (start with the freemarker templates) and see what text transformation functions are used there. You should be able to just replicate these in the other template.

I don't have the code handy but if you can't find the right template let me know and I will find it when I have access.

eitzenbe commented 2 months ago

mvn install -DskipTests running ;)

wakaleo commented 2 months ago

Yep, you can test a snapshot build that way (don't run the full test suite each time, as the full test suite takes a while and is a bit Linux-centric for some of the tests).

eitzenbe commented 2 months ago

I am off course on linux ;)

eitzenbe commented 2 months ago

@wakaleo PR avail for review, I did a minimal invasive fix, which might not be the most elegant way, but I don't want to open up pandoras box here ;) I tested with scenario titles containing <,>," as these would fuck the report HTML the most and it seems you are checking for a lot of special chars but not these

eitzenbe commented 2 months ago

@wakaleo Tried to reach out to you but reachme@.... seems to be not working

This is the mail system at host eitzen.at.

I'm sorry to have to inform you that your message could not be delivered to one or more recipients. It's attached below.

For further assistance, please send mail to postmaster.

If you do so, please include this problem report. You can delete your own text from the attached returned message.

               The mail system

reachme@wakaleo.com: host aspmx.l.google.com[173.194.76.27] said: 550-5.1.1 The email account that you tried to reach does not exist. Please try 550-5.1.1 double-checking the recipient's email address for typos or 550-5.1.1 unnecessary spaces. For more information, go to 550 5.1.1 https://support.google.com/mail/?p=NoSuchUser 5b1f17b1804b1-42bb6deacbdsi70759145e9.19 - gsmtp (in reply to RCPT TO command)

wakaleo commented 2 months ago

This should be good - I will take a look this evening

eitzenbe commented 2 months ago

sent message via l...in

wakaleo commented 1 month ago

There are some failing tests in WhenFormattingDataForTheHTMLReports.groovy and WhenFormattingForHTML.java - they need to be updated to reflect the new behaviour, or in some cases they may indicate regressions in other behaviour that need to be integrated in your code. You can run these tests in isolation

e.g.

image
eitzenbe commented 1 month ago

I will check tomorrow

04.09.2024 20:32:00 John Ferguson Smart @.***>:

There are some failing tests in WhenFormattingDataForTheHTMLReports.groovy and WhenFormattingForHTML.java - they need to be updated to reflect the new behaviour, or in some cases they may indicate regressions in other behaviour that need to be integrated in your code. You can run these tests in isolation

e.g. image.png (view on web)[https://github.com/user-attachments/assets/15deb18a-13b4-4f39-a617-767943aaa066]

— Reply to this email directly, view it on GitHub[https://github.com/serenity-bdd/serenity-core/issues/3522#issuecomment-2329724715], or unsubscribe[https://github.com/notifications/unsubscribe-auth/ABKNTSDOAEAGQTEDQ5D5QMDZU5GZ3AVCNFSM6AAAAABNRTHZAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRZG4ZDINZRGU]. You are receiving this because you authored the thread. [Verfolgungsbild][https://github.com/notifications/beacon/ABKNTSDBABQQYJX2N7FU7OTZU5GZ3A5CNFSM6AAAAABNRTHZAOWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUK3TDSW.gif]

eitzenbe commented 1 month ago

@wakaleo Hmmm the method htmlCompatible in the formatter class should escape all special characters assuming it is a text that is put in or? If so the test case is wrong, without javadoc its hard to see whether the test is wrong or the new method impl I did. From the name "plainHtmlCompatible" I deducted that < and > characters should be escaped too as these could lead to problems in the title section of scenarios in the report.

The test method name implies you are actually testing the renderMarkdown method in which case I would just remove the chained call to htmlCompatible., What do you say?

br Thomas

wakaleo commented 1 month ago

The test method names are usually a good indication of the indented behaviour. Sometimes we don't want to escape < and > because we actually do want to render the HTML as is (particularly if we are allowing markdown rendering such as is the case for test titles and descriptions). For example, this one looks like there is an issue rendering the markdown notation:

image
eitzenbe commented 1 month ago

reworking the code to not touch the formatter classes at all, PR incoming ;)

eitzenbe commented 1 month ago

Ok so fixed it with staying inside ftl to avoid side effects, but WHY are you replacing "<" in example table values with "{" this may cause "havoc" if test data on purpose contains a "<"... Not going down that rabbit whole but might be a good idea to rework that at a later time ;)

image

wakaleo commented 1 month ago

Ok so fixed it with staying inside ftl to avoid side effects, but WHY are you replacing "<" in example table values with "{" this may cause "havoc" if test data on purpose contains a "<"... Not going down that rabbit whole but might be a good idea to rework that at a later time ;)

image

Thanks. This is a good point and there was probably a very good reason at the time which I don't recall now 🤷

Did you raise a PR?