Html report improvements #30094

amankagithub · 2024-03-25T14:15:25Z

Copy annotation text by one click .
Search test case based on annotations .

pavelfeldman · 2024-03-25T21:47:31Z

Please start with filing a feature request, even if you plan to contribute this PR.

pavelfeldman

Overall, looks good. There are some comments I'd like you to address and we are missing the tests for the new functionality.

pavelfeldman · 2024-03-27T15:39:10Z

packages/html-reporter/src/filter.ts

@@ -114,6 +120,7 @@ export class Filter {
        line: String(test.location.line),
        column: String(test.location.column),
        labels: test.tags.map(tag => tag.toLowerCase()),
+        annotations: test.annotations.map(a => a.type.toLowerCase() + ':' + a.description?.toLocaleLowerCase())


To make sure you only use : once in the filter.

Suggested change

annotations: test.annotations.map(a => a.type.toLowerCase() + ':' + a.description?.toLocaleLowerCase())

annotations: test.annotations.map(a => a.type.toLowerCase() + '=' + a.description?.toLocaleLowerCase())

pavelfeldman · 2024-03-27T15:41:04Z

packages/html-reporter/src/filter.ts

@@ -144,7 +151,14 @@ export class Filter {
      if (!matches)
        return false;
    }
-
+    if (this.annotations.length) {


This will return all the tests w/o annotations as matching the search criteria.

I tried and it did not . Only test case with annotations will be returned . In below example ,only two test cases which had some annotation is displayed . Am I missing something here?

Ah, you are right, I missed that this is a filter here. Still need a test for every new feature though.

I have added the test cases for copy feature . Please review .

pavelfeldman · 2024-03-27T15:47:17Z

packages/html-reporter/src/testCaseView.tsx

  return (
-    <div className='test-case-annotation'>
-      <span style={{ fontWeight: 'bold' }}>{type}</span>
+    <div className='test-case-annotation' onMouseEnter={onHover} onMouseLeave={onLeave}>


It looks like you are re-rendering the component that you add listeners to on hover, which might results in unexpected results.

I'd suggest adding a new component next to CopyToClipboard called aaa that would allow copying its content. That way we can use it in more places. I would also recommend that the visibility is implemented using CSS.

Not fully sure what you mean here . Made the required changes based on my what I could understand from your comment .

yury-s · 2024-04-15T23:15:28Z

packages/html-reporter/src/filter.ts

-
+    if (this.annotations.length) {
+      const matches = this.annotations.every(annotation =>
+        searchValues.annotations.some(_annotation => (


style nit: either drop () around _annotation.includes(annotation) or use {}

yury-s · 2024-04-15T23:17:46Z

packages/html-reporter/src/filter.ts

+          _annotation.includes(annotation)
+        )));
+      if (!matches)
+        return false;


Please add a test to tests/playwright-test/reporter-html.spec.ts that checks that the annotation filter actually works.

Have added the test as you suggested . Please review .

github-actions · 2024-04-25T11:14:36Z

Test results for "tests 1"

27486 passed, 672 skipped
✔️✔️✔️

Merge workflow run.

dgozman · 2024-05-08T17:35:06Z

@amankagithub Unfortunately, we found it hard to review and merge this PR. Therefore, it is going slowly, sorry for that!

To make this process better, let's follow our general guidelines:

Start with filing a feature request following the issue template.
Split this PR into multiple focused PRs, e.g. "copy to clipboard" vs "accessibility improvements" vs "filter by annotations".
For any UI change, attach a screenshot that illustrates the change.

I'll close this PR, since it won't be merged due to the above.