- 
                Notifications
    
You must be signed in to change notification settings  - Fork 5.4k
 
test: Support window switching without WebSocket background connection #34772
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
test: Support window switching without WebSocket background connection #34772
Conversation
          
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/qa (1 files, +4 -1)
  | 
    
| 
               | 
          ||
| while (timeElapsed <= this.timeout) { | ||
| for (const handle of windowHandles) { | ||
| // Wait 25 x 200ms = 5 seconds for the title to match the target title | 
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 implementation I went with here for the non-socket-based "find window with title" code was based on the version we used prior to the WebSocket connection to the background:
| async switchToWindowWithTitle( | 
It seemed a bit nicer than the switchToWindowByTitleWithoutSocket implementation. Though I did opt for the delay and retry settings from switchToWindowByTitleWithoutSocket, as they seemed more reasonable (25 retries with 200ms delay versus 8 retries with 2500ms delay. 2500 is way too long a delay)
30737d1    to
    31e14ca      
    Compare
  
    
          Builds ready [31e14ca]
 UI Startup Metrics (1227 ± 58 ms)
 Benchmark value 4 exceeds gate value 1 for chrome browserify home mean initialActions Benchmark value 832 exceeds gate value 830 for chrome browserify home mean loadScripts Benchmark value 235 exceeds gate value 18 for chrome browserify home p95 backgroundConnect Benchmark value 12 exceeds gate value 1.2 for chrome browserify home p95 initialActions Benchmark value 2340 exceeds gate value 2192 for chrome webpack home mean uiStartup Benchmark value 1876 exceeds gate value 1711 for chrome webpack home mean load Benchmark value 1859 exceeds gate value 1704 for chrome webpack home mean domContentLoaded Benchmark value 59 exceeds gate value 40 for chrome webpack home mean backgroundConnect Benchmark value 1854 exceeds gate value 1699 for chrome webpack home mean loadScripts Benchmark value 2647 exceeds gate value 2454 for chrome webpack home p95 uiStartup Benchmark value 2268 exceeds gate value 2030 for chrome webpack home p95 load Benchmark value 2105 exceeds gate value 2005 for chrome webpack home p95 domContentLoaded Benchmark value 70 exceeds gate value 57 for chrome webpack home p95 domInteractive Benchmark value 296 exceeds gate value 90 for chrome webpack home p95 backgroundConnect Benchmark value 8 exceeds gate value 7 for chrome webpack home p95 initialActions Benchmark value 2101 exceeds gate value 1970 for chrome webpack home p95 loadScripts Benchmark value 1460 exceeds gate value 1405 for firefox browserify home mean uiStartup Benchmark value 1262 exceeds gate value 1245 for firefox browserify home mean load Benchmark value 1261 exceeds gate value 1239 for firefox browserify home mean domContentLoaded Benchmark value 28 exceeds gate value 25 for firefox browserify home mean backgroundConnect Benchmark value 26 exceeds gate value 25 for firefox browserify home mean firstReactRender Benchmark value 4 exceeds gate value 1 for firefox browserify home mean initialActions Benchmark value 1240 exceeds gate value 1230 for firefox browserify home mean loadScripts Benchmark value 1704 exceeds gate value 1660 for firefox browserify home p95 uiStartup Benchmark value 206 exceeds gate value 195 for firefox browserify home p95 domInteractive Benchmark value 37 exceeds gate value 24 for firefox browserify home p95 getState Benchmark value 16 exceeds gate value 2 for firefox browserify home p95 initialActions Benchmark value 32 exceeds gate value 26 for firefox webpack home mean backgroundConnect Benchmark value 54 exceeds gate value 38 for firefox webpack home mean firstReactRender Benchmark value 6 exceeds gate value 1 for firefox webpack home mean initialActions Benchmark value 238 exceeds gate value 156 for firefox webpack home p95 domInteractive Benchmark value 62 exceeds gate value 50 for firefox webpack home p95 firstReactRender Benchmark value 21 exceeds gate value 2 for firefox webpack home p95 initialActions Sum of mean exceeds: 996ms | Sum of p95 exceeds: 1304.8ms Sum of all benchmark exceeds: 2300.8ms Bundle size diffs [🚨 Warning! Bundle size has increased!]
  | 
    
31e14ca    to
    8d2edc3      
    Compare
  
    
          Builds ready [8d2edc3]
 UI Startup Metrics (1154 ± 77 ms)
 Benchmark value 4 exceeds gate value 1 for chrome browserify home mean initialActions Benchmark value 238 exceeds gate value 18 for chrome browserify home p95 backgroundConnect Benchmark value 12 exceeds gate value 1.2 for chrome browserify home p95 initialActions Benchmark value 2203 exceeds gate value 2192 for chrome webpack home mean uiStartup Benchmark value 2561 exceeds gate value 2454 for chrome webpack home p95 uiStartup Benchmark value 66 exceeds gate value 57 for chrome webpack home p95 domInteractive Benchmark value 388 exceeds gate value 370 for chrome webpack home p95 firstReactRender Benchmark value 8 exceeds gate value 7 for chrome webpack home p95 initialActions Benchmark value 264 exceeds gate value 65 for chrome webpack home p95 setupStore Benchmark value 31 exceeds gate value 25 for firefox browserify home mean backgroundConnect Benchmark value 26 exceeds gate value 25 for firefox browserify home mean firstReactRender Benchmark value 4 exceeds gate value 1 for firefox browserify home mean initialActions Benchmark value 219 exceeds gate value 195 for firefox browserify home p95 domInteractive Benchmark value 74 exceeds gate value 70 for firefox browserify home p95 backgroundConnect Benchmark value 29 exceeds gate value 24 for firefox browserify home p95 getState Benchmark value 12 exceeds gate value 2 for firefox browserify home p95 initialActions Benchmark value 31 exceeds gate value 27 for firefox browserify home p95 setupStore Benchmark value 32 exceeds gate value 26 for firefox webpack home mean backgroundConnect Benchmark value 51 exceeds gate value 38 for firefox webpack home mean firstReactRender Benchmark value 4 exceeds gate value 1 for firefox webpack home mean initialActions Benchmark value 253 exceeds gate value 156 for firefox webpack home p95 domInteractive Benchmark value 71 exceeds gate value 49 for firefox webpack home p95 backgroundConnect Benchmark value 62 exceeds gate value 50 for firefox webpack home p95 firstReactRender Benchmark value 11 exceeds gate value 2 for firefox webpack home p95 initialActions Sum of mean exceeds: 260ms | Sum of p95 exceeds: 751.8ms Sum of all benchmark exceeds: 1011.8ms Bundle size diffs [🚨 Warning! Bundle size has increased!]
  | 
    
Our E2E driver class now allows the methods `switchWindow` and `switchToWindowWithTitle` methods to be used in tests that have the Mocha background server disabled (such as those that use a production build, or those that involve the extension resetting). Previously the `switchToWindowWithTitleWithoutSocket` driver method served this purpose, but the test author had to know to use this alternative method. Plus it didn't allow switching to a window handle, or any of the other switch methods that internally use `switchWindow`. There are a couple of switch methods still not supported, but in these cases an error will be thrown so that the failure reason will be clear. This unblocks #31435
8d2edc3    to
    1c69941      
    Compare
  
    
          Builds ready [1c69941]
 UI Startup Metrics (1142 ± 60 ms)
 Benchmark value 2 exceeds gate value 1 for chrome browserify home mean initialActions Benchmark value 231 exceeds gate value 18 for chrome browserify home p95 backgroundConnect Benchmark value 9 exceeds gate value 1.2 for chrome browserify home p95 initialActions Benchmark value 32 exceeds gate value 29 for chrome webpack home mean getState Benchmark value 2475 exceeds gate value 2454 for chrome webpack home p95 uiStartup Benchmark value 61 exceeds gate value 57 for chrome webpack home p95 domInteractive Benchmark value 389 exceeds gate value 370 for chrome webpack home p95 firstReactRender Benchmark value 285 exceeds gate value 195 for chrome webpack home p95 getState Benchmark value 11 exceeds gate value 7 for chrome webpack home p95 initialActions Benchmark value 30 exceeds gate value 25 for firefox browserify home mean backgroundConnect Benchmark value 27 exceeds gate value 25 for firefox browserify home mean firstReactRender Benchmark value 3 exceeds gate value 1 for firefox browserify home mean initialActions Benchmark value 10 exceeds gate value 9 for firefox browserify home mean setupStore Benchmark value 213 exceeds gate value 195 for firefox browserify home p95 domInteractive Benchmark value 37 exceeds gate value 24 for firefox browserify home p95 getState Benchmark value 11 exceeds gate value 2 for firefox browserify home p95 initialActions Benchmark value 45 exceeds gate value 27 for firefox browserify home p95 setupStore Benchmark value 31 exceeds gate value 26 for firefox webpack home mean backgroundConnect Benchmark value 52 exceeds gate value 38 for firefox webpack home mean firstReactRender Benchmark value 5 exceeds gate value 1 for firefox webpack home mean initialActions Benchmark value 253 exceeds gate value 156 for firefox webpack home p95 domInteractive Benchmark value 76 exceeds gate value 49 for firefox webpack home p95 backgroundConnect Benchmark value 64 exceeds gate value 50 for firefox webpack home p95 firstReactRender Benchmark value 38 exceeds gate value 32 for firefox webpack home p95 getState Benchmark value 16 exceeds gate value 2 for firefox webpack home p95 initialActions Sum of mean exceeds: 244ms | Sum of p95 exceeds: 574.8ms Sum of all benchmark exceeds: 818.8ms Bundle size diffs [🚨 Warning! Bundle size has increased!]
  | 
    
          Builds ready [d55acc1]
 UI Startup Metrics (1142 ± 65 ms)
 Benchmark value 24 exceeds gate value 23 for chrome browserify home mean firstReactRender Benchmark value 4 exceeds gate value 1 for chrome browserify home mean initialActions Benchmark value 228 exceeds gate value 18 for chrome browserify home p95 backgroundConnect Benchmark value 49 exceeds gate value 45 for chrome browserify home p95 firstReactRender Benchmark value 15 exceeds gate value 1.2 for chrome browserify home p95 initialActions Benchmark value 2519 exceeds gate value 2454 for chrome webpack home p95 uiStartup Benchmark value 59 exceeds gate value 57 for chrome webpack home p95 domInteractive Benchmark value 389 exceeds gate value 370 for chrome webpack home p95 firstReactRender Benchmark value 253 exceeds gate value 195 for chrome webpack home p95 getState Benchmark value 9 exceeds gate value 7 for chrome webpack home p95 initialActions Benchmark value 26 exceeds gate value 25 for firefox browserify home mean backgroundConnect Benchmark value 6 exceeds gate value 1 for firefox browserify home mean initialActions Benchmark value 243 exceeds gate value 195 for firefox browserify home p95 domInteractive Benchmark value 29 exceeds gate value 2 for firefox browserify home p95 initialActions Benchmark value 32 exceeds gate value 27 for firefox browserify home p95 setupStore Benchmark value 101 exceeds gate value 100 for firefox webpack home mean domInteractive Benchmark value 52 exceeds gate value 38 for firefox webpack home mean firstReactRender Benchmark value 3 exceeds gate value 1 for firefox webpack home mean initialActions Benchmark value 254 exceeds gate value 156 for firefox webpack home p95 domInteractive Benchmark value 67 exceeds gate value 50 for firefox webpack home p95 firstReactRender Benchmark value 11 exceeds gate value 2 for firefox webpack home p95 initialActions Benchmark value 37 exceeds gate value 28 for firefox webpack home p95 setupStore Sum of mean exceeds: 232ms | Sum of p95 exceeds: 586.8ms Sum of all benchmark exceeds: 818.8ms Bundle size diffs [🚨 Warning! Bundle size has increased!]
  | 
    
| 
           @Gudahtt I would rather approach this as changing the   | 
    
| 
           Could you elaborate on why? I found the operations easier to understand as operations just with the Selenium Webdriver. To put them inside the  Also it's not totally clear to me what   | 
    
          Builds ready [fc1081d]
 UI Startup Metrics (1173 ± 73 ms)
 Benchmark value 3 exceeds gate value 1 for chrome browserify home mean initialActions Benchmark value 229 exceeds gate value 18 for chrome browserify home p95 backgroundConnect Benchmark value 14 exceeds gate value 1.2 for chrome browserify home p95 initialActions Benchmark value 2301 exceeds gate value 2192 for chrome webpack home mean uiStartup Benchmark value 1716 exceeds gate value 1711 for chrome webpack home mean load Benchmark value 1709 exceeds gate value 1704 for chrome webpack home mean domContentLoaded Benchmark value 9 exceeds gate value 7 for chrome webpack home mean initialActions Benchmark value 1704 exceeds gate value 1699 for chrome webpack home mean loadScripts Benchmark value 2633 exceeds gate value 2454 for chrome webpack home p95 uiStartup Benchmark value 396 exceeds gate value 370 for chrome webpack home p95 firstReactRender Benchmark value 244 exceeds gate value 195 for chrome webpack home p95 getState Benchmark value 19 exceeds gate value 7 for chrome webpack home p95 initialActions Benchmark value 27 exceeds gate value 25 for firefox browserify home mean backgroundConnect Benchmark value 26 exceeds gate value 25 for firefox browserify home mean firstReactRender Benchmark value 4 exceeds gate value 1 for firefox browserify home mean initialActions Benchmark value 253 exceeds gate value 195 for firefox browserify home p95 domInteractive Benchmark value 29 exceeds gate value 24 for firefox browserify home p95 getState Benchmark value 15 exceeds gate value 2 for firefox browserify home p95 initialActions Benchmark value 34 exceeds gate value 26 for firefox webpack home mean backgroundConnect Benchmark value 53 exceeds gate value 38 for firefox webpack home mean firstReactRender Benchmark value 4 exceeds gate value 1 for firefox webpack home mean initialActions Benchmark value 210 exceeds gate value 156 for firefox webpack home p95 domInteractive Benchmark value 100 exceeds gate value 49 for firefox webpack home p95 backgroundConnect Benchmark value 68 exceeds gate value 50 for firefox webpack home p95 firstReactRender Benchmark value 15 exceeds gate value 2 for firefox webpack home p95 initialActions Benchmark value 34 exceeds gate value 28 for firefox webpack home p95 setupStore Sum of mean exceeds: 367ms | Sum of p95 exceeds: 707.8ms Sum of all benchmark exceeds: 1074.8ms Bundle size diffs [🚨 Warning! Bundle size has increased!]
  | 
    
          
 I looked at it in more detail, and I think I would move all window-related commands into the class and maybe rename it WindowSwitcher. Also I realized it's a little confusing that both of these exist: 
 If I get some time, maybe I'll do this later.  | 
    
| 
           That sounds like a good solution to me  | 
    
| await driver.switchToWindowByTitleWithoutSocket( | ||
| WINDOW_TITLES.ExtensionUpdating, | ||
| ); | ||
| await driver.switchToWindowWithTitle(WINDOW_TITLES.ExtensionUpdating); | 
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.
that's a great improvement ty!
Description
Our E2E driver class now allows the methods
switchWindowandswitchToWindowWithTitlemethods to be used in tests that have the WebSocket connection to the background process disabled (such as those that use a production build, or those that involve the extension resetting).Previously the
switchToWindowWithTitleWithoutSocketdriver method served this purpose, but the test author had to know to use this alternative method. Plus it didn't allow switching to a window handle, or any of the other switch methods that internally useswitchWindow.There are a couple of switch methods still not supported, but in these cases an error will be thrown so that the failure reason will be clear.
Changelog
CHANGELOG entry: null
Related issues
This unblocks #31435
Manual testing steps
N/A
Screenshots/Recordings
N/A
Pre-merge author checklist
Pre-merge reviewer checklist