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

[Feature]: Add a parameter in the test.step() method to hide code in report #34052

Open
Sedatyf opened this issue Dec 17, 2024 · 9 comments
Open

Comments

@Sedatyf
Copy link

Sedatyf commented Dec 17, 2024

🚀 Feature Request

I just discovered the test.step feature. It's really nice and helps me having a nice and tidy report. Unfortunately, every test.step() even the one in Typescript decorator are showing code snippets.
This results in a weird and messy report at the end.
I'm suggesting to add a parameter whether to decide if we want it or not in the HTML report. Or disable the code snippets for test.step by default

Example

This example show the wrong behavior. The right example would be just a dropdown with the step name in it
Image

Motivation

I think test.step is a great feature for keeping an HTML report tidy, without using any BDD/Cucumber methods in the project. But with the code displayed, it looks a bit complicated and unreadable, which defeats the whole purpose of the feature.

@vitalets
Copy link
Contributor

@pavelfeldman What if allow passing location: null to test.step() and then don't render the code snippet?

await test.step('launch browser', () => {
  // ...
}, { location: null });

@ZDGN-for-Git
Copy link

Totally agree with @Sedatyf.
Do like this feature for my reports but what a mess to have the code detail.
May we have at least a reporter option to deactivate ✂️

@EliW7
Copy link

EliW7 commented Jan 28, 2025

I'm working on this

@asherdrake
Copy link

@pavelfeldman What if allow passing location: null to test.step() and then don't render the code snippet?

await test.step('launch browser', () => {
// ...
}, { location: null });

Hello @vitalets , I am working with @EliW7 on this issue.

What is your motivation for using location: null to disable rendering code snippets? I believe that adding a new parameter to test.step() for toggling code snippets would work better, since users might confuse location: null with the default behavior of location: undefined.

@vitalets
Copy link
Contributor

What is your motivation for using location: null to disable rendering code snippets? I believe that adding a new parameter to test.step() for toggling code snippets would work better, since users might confuse location: null with the default behavior of location: undefined.

Hi @asherdrake ! My motivation is to keep step API more concise. Re-using existing option instead of introducing a new one. Strictly speaking, location: undefined and location: null express exactly what we need:

  • location: undefined: I didn't define location, use default one
  • location: null: I explicitly set location value to null, don't render anything

But I agree on your point too. It may be confusing, as null and undefined are often treated as the same thing. I think the final decision is up to Playwright team.

Just for visual comparison:

// re-using `location`
await test.step('launch browser', () => {
  // ...
}, { location: null });

// using new option e.g. `snippet`
await test.step('launch browser', () => {
  // ...
}, { snippet: false });

@vitalets
Copy link
Contributor

Maybe it's better to not change test.step() api at all, but make it a setting on the reporter level.
Imagine the situation when I don't what to see snippets in the HTML reporter, but I still need them in my custom reporter for other purposes.
I think it can be done in two ways:

  • a checkbox in the HTML reporter ui, e.g. "show snippets"
  • a setting in the HTML reporter options, e.g. reporter: [['html', { snippets: false }]]

@ZDGN-for-Git
Copy link

@vitalets
Regarding the documentation, using the location option may be a great solution:

Specifies a custom location for the step to be shown in test reports and trace viewer. By default, location of the test.step() call is shown.

May be using a wording as unshown, hidden, or something close could be explicit enough.

@asherdrake
Copy link

Maybe it's better to not change test.step() api at all, but make it a setting on the reporter level. Imagine the situation when I don't what to see snippets in the HTML reporter, but I still need them in my custom reporter for other purposes. I think it can be done in two ways:

  • a checkbox in the HTML reporter ui, e.g. "show snippets"
  • a setting in the HTML reporter options, e.g. reporter: [['html', { snippets: false }]]

@vitalets
Right now I am leaning towards this checkbox solution.
After investigating the location option, it seems that the "file" and "line" properties are used to create the file path that is shown to the right of each step name. Messing with location could mean interfering with that file path being displayed.

Additionally, it seems that the function that creates the code snippets and adds them to a test step is quite disconnected from the implementation of test.step(). I am finding it difficult to come up with a way to toggle code snippets via a parameter from test.step()'s implementation, that does not disable code snippets for tests unrelated to test.step() or does not require changes across multiple files.

When you describe a checkbox in the UI to toggle snippets, is it one checkbox for all snippets in the report, or a checkbox for each individual snippet?

@vitalets
Copy link
Contributor

When you describe a checkbox in the UI to toggle snippets, is it one checkbox for all snippets in the report, or a checkbox for each individual snippet?

I'd personally vote for a global checkbox, that toggles snippets visibility for the whole report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants