Skip to content

feat(reporters): Add error position to JSON Report#9151

Merged
dgozman merged 5 commits into
microsoft:masterfrom
sidharthv96:feat/jsonErrorPosition
Sep 30, 2021
Merged

feat(reporters): Add error position to JSON Report#9151
dgozman merged 5 commits into
microsoft:masterfrom
sidharthv96:feat/jsonErrorPosition

Conversation

@sidharthv96
Copy link
Copy Markdown
Contributor

@mxschmitt this is the fix for the initial problem with #9147

Will raise the PR for annotations soon.

Comment thread src/test/reporters/json.ts Outdated
Comment thread src/test/reporters/json.ts Outdated
body?: string;
contentType: string;
}[];
line?: number;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's instead do errorLocation?: { line: number, column: number }.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, was following the convention in

export interface JSONReportSuite {
title: string;
file: string;
column: number;
line: number;
specs: JSONReportSpec[];
suites?: JSONReportSuite[];
}
export interface JSONReportSpec {
tags: string[],
title: string;
ok: boolean;
tests: JSONReportTest[];
file: string;
line: number;
column: number;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@dgozman which approach should we take?
I'd prefer the errorLocation approach, but that would deviate from the convention for the parents of that object.

The way I've seen in jest report is a position object with line and col like you suggested.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I feel strongly that test result does not have a line/column because it is not defined in the source like suite or test. errorLocation works nicely here, in line with https://playwright.dev/docs/api/class-location.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Just curious, shouldn't JSONReportSuite and JSONReportSpec be using the Location class as file, line and column are being used there?

@sidharthv96 sidharthv96 requested a review from dgozman September 28, 2021 05:24
Comment thread src/test/reporters/base.ts Outdated
Comment thread src/test/reporters/json.ts Outdated
body?: string;
contentType: string;
}[];
line?: number;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I feel strongly that test result does not have a line/column because it is not defined in the source like suite or test. errorLocation works nicely here, in line with https://playwright.dev/docs/api/class-location.

Copy link
Copy Markdown
Collaborator

@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.

Looks great, thank you for the PR!

@dgozman dgozman merged commit fcb7d2b into microsoft:master Sep 30, 2021
sidharthv96 added a commit to sidharthv96/playwright that referenced this pull request Oct 1, 2021
* upstream/master: (21 commits)
  fix(test runner): do not write missing snapshot until the last retry (microsoft#9246)
  feat(reporters): Add error position to JSON Report (microsoft#9151)
  feat(fetch): import/export storageState (microsoft#9244)
  fix: allow binary response interception (microsoft#9236)
  test(cookies): add a test for SameSite=None cookies (microsoft#9242)
  feat(webkit): roll WebKit to 1550 (microsoft#9239)
  test: add test for downloading PDF files (microsoft#9235)
  fix: leaking route handlers when times is used in Page.route (microsoft#9234)
  chore: upgrade commander.js to version 8 (microsoft#9230)
  feat(fetch): store cookies between requests (microsoft#9221)
  fix(selenium connect): register in gracefullyCloseAll for driver cleanup (microsoft#9218)
  fix(toBeHidden): return true to missing elements (microsoft#9205)
  chore: fix PlaywrightClient disconnection logic (microsoft#9149)
  fix(expect): beautiful expect stacks (microsoft#9204)
  feat(fetch): support ignoreHTTPSErrors option (microsoft#9206)
  feat(api): introduce locator.waitFor (microsoft#9200)
  feat(fetch): send Playwright as default user-agent for global fetch (microsoft#9195)
  feat(test runner): collect test error from worker teardown (microsoft#9190)
  test: add tests for Cross-Origin-Opener-Policy navigation (microsoft#9184)
  test: get response body for COOP responses (microsoft#9196)
  ...
sidharthv96 added a commit to sidharthv96/playwright that referenced this pull request Oct 1, 2021
…tionWithBaseChange

* upstream/master: (21 commits)
  fix(test runner): do not write missing snapshot until the last retry (microsoft#9246)
  feat(reporters): Add error position to JSON Report (microsoft#9151)
  feat(fetch): import/export storageState (microsoft#9244)
  fix: allow binary response interception (microsoft#9236)
  test(cookies): add a test for SameSite=None cookies (microsoft#9242)
  feat(webkit): roll WebKit to 1550 (microsoft#9239)
  test: add test for downloading PDF files (microsoft#9235)
  fix: leaking route handlers when times is used in Page.route (microsoft#9234)
  chore: upgrade commander.js to version 8 (microsoft#9230)
  feat(fetch): store cookies between requests (microsoft#9221)
  fix(selenium connect): register in gracefullyCloseAll for driver cleanup (microsoft#9218)
  fix(toBeHidden): return true to missing elements (microsoft#9205)
  chore: fix PlaywrightClient disconnection logic (microsoft#9149)
  fix(expect): beautiful expect stacks (microsoft#9204)
  feat(fetch): support ignoreHTTPSErrors option (microsoft#9206)
  feat(api): introduce locator.waitFor (microsoft#9200)
  feat(fetch): send Playwright as default user-agent for global fetch (microsoft#9195)
  feat(test runner): collect test error from worker teardown (microsoft#9190)
  test: add tests for Cross-Origin-Opener-Policy navigation (microsoft#9184)
  test: get response body for COOP responses (microsoft#9196)
  ...
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.

2 participants