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

feat(trace viewer): Adds _debugName BrowserContextOption to let users define a name for their contexts #5205

Merged
merged 1 commit into from Jan 28, 2021

Conversation

domderen
Copy link
Contributor

This change is adding a new property on the BrowserContextOptions class called _debugName. This property allows defining a user-friendly name for the browser context, and currently it is being used in one place, the Trace Viewer. When user provides the new value in the following way:

const { chromium } = require('playwright');

(async () => {
  const browser = await chromium.launch();
  const context = await browser.newContext({ _traceDir: __dirname, _debugName: 'My custom testcase name' });
  await context.close();
  await browser.close();
})();

The _debugName will be saved in the *.trace file for this browser context, on the context-created event, under the key debugName.

Later, when such a trace is displayed using Trace Viewer, the debugName will be displayed in the dropdown in the top right part of the app instead of the actual trace filename.

Fixes #5157.

Zrzut ekranu 2021-01-28 o 13 52 59

… define a name for their contexts.

This change is adding a new property on the BrowserContextOptions class called `_debugName`. This property allows defining a user-friendly name for the browser context, and currently it is being used in one place, the Trace Viewer. When user provides the new value in the following way:

```typescript
const { chromium } = require('playwright');

(async () => {
  const browser = await chromium.launch();
  const context = await browser.newContext({ _traceDir: __dirname, _debugName: 'My custom testcase name' });
  await context.close();
  await browser.close();
})();
```

The `_debugName` will be saved in the `*.trace` file for this browser context, on the `context-created` event, under the key `debugName`.

Later, when such a trace is displayed using Trace Viewer, the `debugName` will be displayed in the dropdown in the top right part of the app instead of the actual trace filename.

Fixes microsoft#5157.
@domderen
Copy link
Contributor Author

Hey, two things to clarify here:

  1. I went with the _debugName rather than debugName to follow the pattern set by _traceDir. I'm assuming it means that this option is not yet public and documented. Happy to change it, if this one is not that important at the moment.
  2. In the tests file I saw this:
it('should record trace', (test, { browserName, platform }) => {
  test.fixme();
}, async ({browser, testInfo, server}) => {
  ...
});

This looks like those tests are disabled on purpose at the moment. I added a new test checking the _debugName option, but annotated it with the same fixme call for now. Should those be enabled now?

@domderen
Copy link
Contributor Author

Oh I also didn't add the docs yet, again not sure if those get added once the functionality is ready for deployment, or if it should be added in this PR.

@dgozman dgozman changed the title feat(trace viewer): Adds _debugName BrowserContextOption to let users define a name for their contexts. feat(trace viewer): Adds _debugName BrowserContextOption to let users define a name for their contexts Jan 28, 2021
Copy link
Contributor

@dgozman dgozman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

@@ -111,3 +113,19 @@ it('should record trace with POST', (test, { browserName, platform }) => {
expect(fs.existsSync(path.join(traceDir, 'resources', resourceEvent.requestSha1))).toBe(true);
expect(fs.existsSync(path.join(traceDir, 'resources', resourceEvent.responseSha1))).toBe(true);
});

it('should record trace with a debugName', (test, { browserName, platform }) => {
test.fixme();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracing tests are flaky on the bots, I still need to figure out why and reenable them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by bots?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean our GitHub actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok thanks

@@ -373,6 +373,7 @@ BrowserType:
- no-preference
acceptDownloads: boolean?
_traceDir: string?
_debugName: string?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Underscore is good. We'll make it public once we are sure about the api. No need for docs yet.

@dgozman dgozman merged commit f8fbfe2 into microsoft:master Jan 28, 2021
@domderen domderen deleted the browser-context-debug-name branch January 28, 2021 19:11
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.

[Feature] Ability to provide user-defined name for Tracers
2 participants