- 
                Notifications
    You must be signed in to change notification settings 
- Fork 4.8k
feat(html): order by duration #37864
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
Add ability to sort test results in the HTML reporter using query syntax: - `o:duration` sorts by duration ascending - `!o:duration` sorts by duration descending - Supports multiple sort criteria for priority sorting 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Remove unnecessary Sets and spread operations - Push directly onto filter arrays - Move tokenize call into for loop header - Replace continue statements with if/else chain 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Change sortTests to return void and mutate the array in place instead of creating a copy. This is more efficient and clearer. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Update sorting test to include multiple files to verify that sorting works across all tests globally, not just within each file. This test currently fails, exposing the bug that sorting is only applied per-file rather than across all tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
When sorting is active, group all tests into a single anonymous group to maintain the global sort order. File/describe grouping would break the sorting since tests would be displayed grouped by file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace toHaveText assertions with toMatchAriaSnapshot for more comprehensive testing that verifies the entire UI structure. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Reduce snapshot verbosity by only checking buttons, regions, and test links - the parts we actually care about for sorting verification. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Replace page.locator('body') with page.getByRole('main') to more
precisely target the test content area.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
    |  | ||
| static parse(expression: string): Filter { | ||
| const tokens = Filter.tokenize(expression); | ||
| const project = new Set<FilterToken>(); | 
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.
the sets got useless through #35390.
| result.tests.push(...tests); | ||
| } | ||
|  | ||
| if (filter.sort.length) { | 
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.
Let's do a separate createSortedNoFilesModel() method, because it completely disregards whatever the two existing methods do.
        
          
                packages/html-reporter/src/filter.ts
              
                Outdated
          
        
      | tests.sort((a, b) => { | ||
| for (const sortToken of this.sort) { | ||
| let comparison = 0; | ||
| if (sortToken.name === 'duration') | 
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.
While we are here, let's support more sort properties right away?
        
          
                packages/html-reporter/src/filter.ts
              
                Outdated
          
        
      | if (comparison !== 0) | ||
| return sortToken.not ? -comparison : comparison; | ||
| } | ||
| return 0; | 
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.
Let's fall back to some default sorting in this case, to make it deterministic.
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.
Javascript's sorting is stable, doesn't that make it deterministic? https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#sort_stability
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.
I mean it's synchronous, so it's most likely deterministic even if it wasn't stable. So deterministic isn't what you're asking for. Keeping the existing order sounds fine by me though.
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
Add support for sorting tests by duration in the HTML reporter using the query syntax: - `o:duration` - sort by duration, slowest tests first - `!o:duration` - sort by duration, fastest tests first When sorting is active, file grouping is disabled and all tests are displayed in a single list sorted according to the specified criteria. The implementation supports multiple sort criteria for priority sorting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Extend sorting to support multiple fields beyond duration: - `o:title` - sort by test title alphabetically - `o:status` - sort by test status (expected/failed/flaky/skipped) - `o:file` - sort by file path - `o:line` - sort by line number - `o:project` - sort by project name All fields support the `!` prefix for reversed sorting. Multiple sort criteria can be combined for priority sorting (e.g., `o:file o:line`). Reuses `cacheSearchValues` for status, file, and project fields to improve performance. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Rename test to "should support sorting" to reflect broader coverage - Add tests for o:title (alphabetical sorting) - Add tests for o:file (file path sorting) - Increase timeout values to multiples of 100ms to prevent flakiness 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
      
        
              This comment has been minimized.
        
        
      
    
  This comment has been minimized.
| How do you order files in that case? Ah, you seem to be merging everything. Curious how it looks for this link: https://mspwblobreport.z1.web.core.windows.net/run-18531305882-1-63d41d2ae75a0e8e26993dce9d827ed0608dcfee/index.html | 
| There you go: https://playwright-html-ordering.netlify.app/#?q=o:duration It loads quite a while, as it's sorting 47k rows. | 
| 
 This is unusable. We should think about some type of grouping, so that you never expand 47k tests to end up with 800k DOM nodes on the page. | 
| Some options that come to mind: 
 I know which one I prefer. | 
| I updated it to show chunks of a 100, how do you like it? | 
| Test results for "MCP"2570 passed, 108 skipped Merge workflow run. | 
| Test results for "tests 1"3 flaky47089 passed, 815 skipped Merge workflow run. | 
| Do we land it or do we shelve it? | 
| This particular one goes to the shelf, but there'll be other! | 
Adds an

o:durationfilter that allows seeing the slowest tests quickly:Part of #37724.