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]: Adding Steps to Trace via Playwright API #33047

Closed
Snooz82 opened this issue Oct 10, 2024 · 11 comments · Fixed by #33450
Closed

[Feature]: Adding Steps to Trace via Playwright API #33047

Snooz82 opened this issue Oct 10, 2024 · 11 comments · Fixed by #33450
Labels
open-to-a-pull-request The feature request looks good, we are open to reviewing a PR

Comments

@Snooz82
Copy link
Contributor

Snooz82 commented Oct 10, 2024

🚀 Feature Request

hey y'all,

I am one of the maintainers of the Robot Framework Playwright integration.
I would like to start and stop steps in Tracing via Playwright API. (without test)
Image

I mean these steps that are added in Playwright test with test.step

This is a very cool Unique Selling Point of Playwright compared to others like cypress, etc..

We are not using Playwright.test, because the actual test runner is Robot Framework. But we would like to add the test steps, which are calling Playwright actions, executed in Robot Framework to the tracing. That would massively help analysing the logs on longer system level tests.
How do other Playwright implementation do that?

i think this would be an extremely helpful feature for other language implementation like Playwright Python as well.

Thank you!

Kind regards from the thousands of Playwright users from the Robot Framework Community.

Cheers
René

Example

all language implementation could offer steps to their testing frameworks.

one huge disadvantage for higher level tests of i.e. cypress is, that you do see only all these click, fill and so on in the logs.
For system tests or end2end tests this is really bad.
Playwright Test has a huge advantage there.

Motivation

we want to show the Robot Framework keywords in Trace viewers so that users know where it failed not only in robot logs but in trace.

@Snooz82 Snooz82 changed the title [Feature]: [Feature]: Adding Steps to Trace via Playwright API Oct 10, 2024
@pavelfeldman
Copy link
Member

Could you propose the API here so that we could agree on it. If things work, we'll mark it as open-to-a-pr.

@Snooz82
Copy link
Contributor Author

Snooz82 commented Oct 10, 2024

good question.

i was initially thinking of something like Tracing.startStep(title, options)
and
Tracing.stopStep()

not sure if startStep should somehow return something like an identifier. or indentation level.
so that stop could stop multiple steps at the same time.

so start would increase the indentation level by one with the title and the location in the options.

@pavelfeldman
Copy link
Member

What's in the options? console API has console.group(title) and console.groupEnd(), might sound a bit better than stopStep since it does not really stop the step. Could you flush it out in greater detail, i.e. complete API and semantics. It would affect the @playwright/test and its tracing + its reporters. Would be interesting how this is going to be supported. That'll be quite a tricky change, let's put as much detail as possible here to see if it is worth it.

@Snooz82
Copy link
Contributor Author

Snooz82 commented Oct 10, 2024

I like the idea of being similar to console.group api!
That sounds like a good approach!

I was actually thinking, that it would be more or less compatible to the current test.step and its arguments.
https://playwright.dev/docs/api/class-test#test-step

it should have name (or label as console.group) and options.

options would need location (which is an awesome addition ❤ by the way).
I am actually not sure if the box field is useful in that case, because in PW-Python or Robot Framework or any other language implementation the console outputs are anyway different and would point to the python or whatever line that failed.

I am really not that deep in the architecture of Playwright.test and tracing, so i can not judge the impact, but i was not thinking, that any change on playwright.test and its reports would be needed. Due to tracing being part of playwright API.

Playwright Python can not create test reports, can it? but tracing in context is possible.
So in my mind it would just be an addition to tracing without touching the others.

API doc:

Tracing

group

The tracing.group() method creates a new inline group in the trace log, causing any subsequent calls to be indented by an additional level, until tracing.groupEnd() is called.

Arguments

  • title string

    Step name.

  • options Object (optional)

    • location Location (optional)

      Specifies a custom location for the group to be shown in trace viewer. By default, location is the location of the call.

groupEnd

The tracing.groupEnd() method exits the current inline group in the trace log.

@Snooz82
Copy link
Contributor Author

Snooz82 commented Oct 11, 2024

This is an example how it could look like with Robot Framework.
https://github.com/user-attachments/assets/4efcc7c8-8bb8-40e3-b08d-b69d8acb10f1

Robot Framework Browser (the Playwright library) is using Playwright NodeJS, because we started already in March 2020.

But i do think that this feature would really also benefit PW Java, Python and .NET!

i.e. If people would use Playwright in Behave or Cucumber or SpecFlow/Reqnroll, they could also create groups in the logs for every behaviour step.

Because Robot Framework is used mostly for higher level tests, like system test, acceptance tests, or BDD, one test case can have easily 80 playwright steps. Then it can be hard to identify the actual "high level step" of your test case in the trace viewer and see which low level playwright calls belong to which step.

@DetachHead
Copy link
Contributor

this issue has also been raised at microsoft/playwright-python#1949

@Snooz82
Copy link
Contributor Author

Snooz82 commented Oct 11, 2024

@DeeplyDiligent that is good. it first needs to be implemented here, afaik, so that Python side can adopt.

so the python community has already an active demand

@Snooz82
Copy link
Contributor Author

Snooz82 commented Oct 11, 2024

@pavelfeldman is that roughly what you have expected as explanation from me?

@pavelfeldman
Copy link
Member

Your proposal looks reasonable.

I am really not that deep in the architecture of Playwright.test and tracing, so i can not judge the impact, but i was not thinking, that any change on playwright.test and its reports would be needed. Due to tracing being part of playwright API.

Depending on the implementation it will either work or not work. But I think a naive implementation that implements tracing.group on the server side would make it work.

Specifies a custom location for the group to be shown in trace viewer. By default, location is none.

By default, location should be the call location. It will happen automatically for a naive implementation as well.

@pavelfeldman pavelfeldman added the open-to-a-pull-request The feature request looks good, we are open to reviewing a PR label Oct 12, 2024
@Snooz82
Copy link
Contributor Author

Snooz82 commented Oct 12, 2024

I now was start thinking about how to implement that and how it should behave.

So the question that is bothering me right now: "Should it be allowed to use tracing.group inside a test.step?"
And should it actually show up?

If i understand it correctly, the test.trace file is used to show the "Actions" tree in case of "Test Tracing".
And because steps are only there, steps do never appear in trace.trace or 0-trace.trace right?

When there is a test.tracing trace viewer takes this and does read frame-snapshots and logs from 0-trace.trace but not the api calls itself. Those are taken from test.tracing.

So tracing.group() would not influence the test.trace at all.
Those would be written to trace.trace and not show up when Playwright-Test is used with the "Test Tracing", right?

@Snooz82
Copy link
Contributor Author

Snooz82 commented Oct 14, 2024

I now played a bit in the code and started drafting the feature.
I hope i am not totally at the wrong place and the implementation goes in to correct direction.

Some thoughts about the behaviour.

  • Failing if Tracing is not active: I think the group and groupEnd methods should NOT raise an exception if tracing is not active, because the framework that may create groups, does not know if someone started tracing and because there is not a simple public way how to check if tracing is active.
  • stop/stopChunk: does close all groups
  • group/groupEnd returning void: i think the behaviour of console.group is fine and there is no benefit returning anything. BUT, if we would return an indentation level the users would know where that group has been created. group would return on which level the group has been created and groupEnd returns which level has been ended. Similar to indent++ and indent--

One Issue i am thinking about is what should happen with groups opened before tracing is started.
So tracing in general starts with opening a new Context, but the first call is everything after that.
So if i.e. we start a group before tracing, so that newPage should be in that group, that must be called after the Context has been created with tracing. This is imho not so super problematic, but a bit odd for users, that may want to create groups already at the start.
A solution could be that there is a buffer of group openings in tracing and once start is called these groups are created instantly.

@pavelfeldman: if the code i have committed here is not absolutely unacceptable it would be cool to have a mentor which could give some quicker feedback and support.
i.e. I have no real clue how your guidelines regarding tests are and maybe someone from the team could support here as well.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-to-a-pull-request The feature request looks good, we are open to reviewing a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants