-
Notifications
You must be signed in to change notification settings - Fork 4.9k
fix(trace): operate trace uri, not path #38566
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
This comment has been minimized.
This comment has been minimized.
packages/playwright-core/src/server/trace/viewer/traceViewer.ts
Outdated
Show resolved
Hide resolved
|
@cpAdm do you want to review this one? We used to pass absolute file path into traceURL and I replaced it with the uri that contains path + timestamp. |
This comment has been minimized.
This comment has been minimized.
6cad8b0 to
72f27a4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
cpAdm
left a comment
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.
Can confirm that these changes fixes the related issue. But whilst add it, I think we should try to solve #38364 as well
| return; | ||
| } | ||
|
|
||
| const traceLocation = [ |
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.
Fetching this resource always fails
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.
What do you mean? No live traces tests would pass if this failed.
Test results for "tests 1"2 failed 1 flaky34366 passed, 692 skipped Merge workflow run. |
Test results for "MCP"5 failed 2709 passed, 116 skipped Merge workflow run. |
Fixes #38551