fix(codegen): do not generate Promise.all (#19182)
Drive-by: fix `download`, `dialog` and `popup` signals to use different variable names in all languages.
This commit is contained in:
		
							parent
							
								
									c547416e24
								
							
						
					
					
						commit
						0be4fa768a
					
				|  | @ -340,8 +340,8 @@ class ContextRecorder extends EventEmitter { | ||||||
|   private _generator: CodeGenerator; |   private _generator: CodeGenerator; | ||||||
|   private _pageAliases = new Map<Page, string>(); |   private _pageAliases = new Map<Page, string>(); | ||||||
|   private _lastPopupOrdinal = 0; |   private _lastPopupOrdinal = 0; | ||||||
|   private _lastDialogOrdinal = 0; |   private _lastDialogOrdinal = -1; | ||||||
|   private _lastDownloadOrdinal = 0; |   private _lastDownloadOrdinal = -1; | ||||||
|   private _timers = new Set<NodeJS.Timeout>(); |   private _timers = new Set<NodeJS.Timeout>(); | ||||||
|   private _context: BrowserContext; |   private _context: BrowserContext; | ||||||
|   private _params: channels.BrowserContextRecorderSupplementEnableParams; |   private _params: channels.BrowserContextRecorderSupplementEnableParams; | ||||||
|  | @ -647,12 +647,14 @@ class ContextRecorder extends EventEmitter { | ||||||
| 
 | 
 | ||||||
|   private _onDownload(page: Page) { |   private _onDownload(page: Page) { | ||||||
|     const pageAlias = this._pageAliases.get(page)!; |     const pageAlias = this._pageAliases.get(page)!; | ||||||
|     this._generator.signal(pageAlias, page.mainFrame(), { name: 'download', downloadAlias: String(++this._lastDownloadOrdinal) }); |     ++this._lastDownloadOrdinal; | ||||||
|  |     this._generator.signal(pageAlias, page.mainFrame(), { name: 'download', downloadAlias: this._lastDownloadOrdinal ? String(this._lastDownloadOrdinal) : '' }); | ||||||
|   } |   } | ||||||
| 
 | 
 | ||||||
|   private _onDialog(page: Page) { |   private _onDialog(page: Page) { | ||||||
|     const pageAlias = this._pageAliases.get(page)!; |     const pageAlias = this._pageAliases.get(page)!; | ||||||
|     this._generator.signal(pageAlias, page.mainFrame(), { name: 'dialog', dialogAlias: String(++this._lastDialogOrdinal) }); |     ++this._lastDialogOrdinal; | ||||||
|  |     this._generator.signal(pageAlias, page.mainFrame(), { name: 'dialog', dialogAlias: this._lastDialogOrdinal ? String(this._lastDialogOrdinal) : '' }); | ||||||
|   } |   } | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -77,7 +77,7 @@ export class JavaLanguageGenerator implements LanguageGenerator { | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     if (signals.download) { |     if (signals.download) { | ||||||
|       code = `Download download = ${pageAlias}.waitForDownload(() -> {
 |       code = `Download download${signals.download.downloadAlias} = ${pageAlias}.waitForDownload(() -> {
 | ||||||
|         ${code} |         ${code} | ||||||
|       });`;
 |       });`;
 | ||||||
|     } |     } | ||||||
|  |  | ||||||
|  | @ -74,33 +74,18 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator { | ||||||
|   });`);
 |   });`);
 | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     const emitPromiseAll = signals.popup || signals.download; |  | ||||||
|     if (emitPromiseAll) { |  | ||||||
|       // Generate either await Promise.all([]) or
 |  | ||||||
|       // const [popup1] = await Promise.all([]).
 |  | ||||||
|       let leftHandSide = ''; |  | ||||||
|     if (signals.popup) |     if (signals.popup) | ||||||
|         leftHandSide = `const [${signals.popup.popupAlias}] = `; |       formatter.add(`const ${signals.popup.popupAlias}Promise = ${pageAlias}.waitForEvent('popup');`); | ||||||
|       else if (signals.download) |  | ||||||
|         leftHandSide = `const [download] = `; |  | ||||||
|       formatter.add(`${leftHandSide}await Promise.all([`); |  | ||||||
|     } |  | ||||||
| 
 |  | ||||||
|     // Popup signals.
 |  | ||||||
|     if (signals.popup) |  | ||||||
|       formatter.add(`${pageAlias}.waitForEvent('popup'),`); |  | ||||||
| 
 |  | ||||||
|     // Download signals.
 |  | ||||||
|     if (signals.download) |     if (signals.download) | ||||||
|       formatter.add(`${pageAlias}.waitForEvent('download'),`); |       formatter.add(`const download${signals.download.downloadAlias}Promise = ${pageAlias}.waitForEvent('download');`); | ||||||
| 
 | 
 | ||||||
|     const prefix = (signals.popup || signals.download) ? '' : 'await '; |  | ||||||
|     const actionCall = this._generateActionCall(action); |     const actionCall = this._generateActionCall(action); | ||||||
|     const suffix = emitPromiseAll ? '' : ';'; |     formatter.add(`await ${subject}.${actionCall};`); | ||||||
|     formatter.add(`${prefix}${subject}.${actionCall}${suffix}`); |  | ||||||
| 
 | 
 | ||||||
|     if (emitPromiseAll) |     if (signals.popup) | ||||||
|       formatter.add(`]);`); |       formatter.add(`const ${signals.popup.popupAlias} = await ${signals.popup.popupAlias}Promise;`); | ||||||
|  |     if (signals.download) | ||||||
|  |       formatter.add(`const download${signals.download.downloadAlias} = await download${signals.download.downloadAlias}Promise;`); | ||||||
| 
 | 
 | ||||||
|     return formatter.format(); |     return formatter.format(); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  | @ -81,17 +81,17 @@ export class PythonLanguageGenerator implements LanguageGenerator { | ||||||
|     let code = `${this._awaitPrefix}${subject}.${actionCall}`; |     let code = `${this._awaitPrefix}${subject}.${actionCall}`; | ||||||
| 
 | 
 | ||||||
|     if (signals.popup) { |     if (signals.popup) { | ||||||
|       code = `${this._asyncPrefix}with ${pageAlias}.expect_popup() as popup_info {
 |       code = `${this._asyncPrefix}with ${pageAlias}.expect_popup() as ${signals.popup.popupAlias}_info {
 | ||||||
|         ${code} |         ${code} | ||||||
|       } |       } | ||||||
|       ${signals.popup.popupAlias} = ${this._awaitPrefix}popup_info.value`;
 |       ${signals.popup.popupAlias} = ${this._awaitPrefix}${signals.popup.popupAlias}_info.value`;
 | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     if (signals.download) { |     if (signals.download) { | ||||||
|       code = `${this._asyncPrefix}with ${pageAlias}.expect_download() as download_info {
 |       code = `${this._asyncPrefix}with ${pageAlias}.expect_download() as download${signals.download.downloadAlias}_info {
 | ||||||
|         ${code} |         ${code} | ||||||
|       } |       } | ||||||
|       download = ${this._awaitPrefix}download_info.value`;
 |       download${signals.download.downloadAlias} = ${this._awaitPrefix}download${signals.download.downloadAlias}_info.value`;
 | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     formatter.add(code); |     formatter.add(code); | ||||||
|  |  | ||||||
|  | @ -624,10 +624,9 @@ test.describe('cli codegen', () => { | ||||||
|     ]); |     ]); | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('JavaScript').text).toContain(` |     expect.soft(sources.get('JavaScript').text).toContain(` | ||||||
|   const [page1] = await Promise.all([ |   const page1Promise = page.waitForEvent('popup'); | ||||||
|     page.waitForEvent('popup'), |   await page.getByRole('link', { name: 'link' }).click(); | ||||||
|     page.getByRole('link', { name: 'link' }).click() |   const page1 = await page1Promise;`);
 | ||||||
|   ]);`);
 |  | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('Java').text).toContain(` |     expect.soft(sources.get('Java').text).toContain(` | ||||||
|       Page page1 = page.waitForPopup(() -> { |       Page page1 = page.waitForPopup(() -> { | ||||||
|  | @ -635,14 +634,14 @@ test.describe('cli codegen', () => { | ||||||
|       });`);
 |       });`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('Python').text).toContain(` |     expect.soft(sources.get('Python').text).toContain(` | ||||||
|     with page.expect_popup() as popup_info: |     with page.expect_popup() as page1_info: | ||||||
|         page.get_by_role("link", name="link").click() |         page.get_by_role("link", name="link").click() | ||||||
|     page1 = popup_info.value`);
 |     page1 = page1_info.value`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('Python Async').text).toContain(` |     expect.soft(sources.get('Python Async').text).toContain(` | ||||||
|     async with page.expect_popup() as popup_info: |     async with page.expect_popup() as page1_info: | ||||||
|         await page.get_by_role("link", name="link").click() |         await page.get_by_role("link", name="link").click() | ||||||
|     page1 = await popup_info.value`);
 |     page1 = await page1_info.value`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('C#').text).toContain(` |     expect.soft(sources.get('C#').text).toContain(` | ||||||
|         var page1 = await page.RunAndWaitForPopupAsync(async () => |         var page1 = await page.RunAndWaitForPopupAsync(async () => | ||||||
|  |  | ||||||
|  | @ -222,39 +222,53 @@ test.describe('cli codegen', () => { | ||||||
|       page.waitForEvent('download'), |       page.waitForEvent('download'), | ||||||
|       page.click('a') |       page.click('a') | ||||||
|     ]); |     ]); | ||||||
|     const sources = await recorder.waitForOutput('JavaScript', 'waitForEvent'); |     await Promise.all([ | ||||||
| 
 |  | ||||||
|     expect.soft(sources.get('JavaScript').text).toContain(` |  | ||||||
|   const context = await browser.newContext();`);
 |  | ||||||
|     expect.soft(sources.get('JavaScript').text).toContain(` |  | ||||||
|   const [download] = await Promise.all([ |  | ||||||
|       page.waitForEvent('download'), |       page.waitForEvent('download'), | ||||||
|     page.getByRole('link', { name: 'Download' }).click() |       page.click('a') | ||||||
|   ]);`);
 |     ]); | ||||||
|  |     const sources = await recorder.waitForOutput('JavaScript', 'download1Promise'); | ||||||
|  | 
 | ||||||
|  |     expect.soft(sources.get('JavaScript').text).toContain(` | ||||||
|  |   const downloadPromise = page.waitForEvent('download'); | ||||||
|  |   await page.getByRole('link', { name: 'Download' }).click(); | ||||||
|  |   const download = await downloadPromise;`);
 | ||||||
|  |     expect.soft(sources.get('JavaScript').text).toContain(` | ||||||
|  |   const download1Promise = page.waitForEvent('download'); | ||||||
|  |   await page.getByRole('link', { name: 'Download' }).click(); | ||||||
|  |   const download1 = await download1Promise;`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('Java').text).toContain(` |  | ||||||
|       BrowserContext context = browser.newContext();`);
 |  | ||||||
|     expect.soft(sources.get('Java').text).toContain(` |     expect.soft(sources.get('Java').text).toContain(` | ||||||
|       Download download = page.waitForDownload(() -> { |       Download download = page.waitForDownload(() -> { | ||||||
|         page.getByRole(AriaRole.LINK, new Page.GetByRoleOptions().setName("Download")).click(); |         page.getByRole(AriaRole.LINK, new Page.GetByRoleOptions().setName("Download")).click(); | ||||||
|       });`);
 |       });`);
 | ||||||
|  |     expect.soft(sources.get('Java').text).toContain(` | ||||||
|  |       Download download1 = page.waitForDownload(() -> { | ||||||
|  |         page.getByRole(AriaRole.LINK, new Page.GetByRoleOptions().setName("Download")).click(); | ||||||
|  |       });`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('Python').text).toContain(` |  | ||||||
|     context = browser.new_context()`);
 |  | ||||||
|     expect.soft(sources.get('Python').text).toContain(` |     expect.soft(sources.get('Python').text).toContain(` | ||||||
|     with page.expect_download() as download_info: |     with page.expect_download() as download_info: | ||||||
|         page.get_by_role("link", name="Download").click() |         page.get_by_role("link", name="Download").click() | ||||||
|     download = download_info.value`);
 |     download = download_info.value`);
 | ||||||
|  |     expect.soft(sources.get('Python').text).toContain(` | ||||||
|  |     with page.expect_download() as download1_info: | ||||||
|  |         page.get_by_role("link", name="Download").click() | ||||||
|  |     download1 = download1_info.value`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('Python Async').text).toContain(` |  | ||||||
|     context = await browser.new_context()`);
 |  | ||||||
|     expect.soft(sources.get('Python Async').text).toContain(` |     expect.soft(sources.get('Python Async').text).toContain(` | ||||||
|     async with page.expect_download() as download_info: |     async with page.expect_download() as download_info: | ||||||
|         await page.get_by_role("link", name="Download").click() |         await page.get_by_role("link", name="Download").click() | ||||||
|     download = await download_info.value`);
 |     download = await download_info.value`);
 | ||||||
|  |     expect.soft(sources.get('Python Async').text).toContain(` | ||||||
|  |     async with page.expect_download() as download1_info: | ||||||
|  |         await page.get_by_role("link", name="Download").click() | ||||||
|  |     download1 = await download1_info.value`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('C#').text).toContain(` |     expect.soft(sources.get('C#').text).toContain(` | ||||||
|         var context = await browser.NewContextAsync();`);
 |         var download = await page.RunAndWaitForDownloadAsync(async () => | ||||||
|  |         { | ||||||
|  |             await page.GetByRole(AriaRole.Link, new() { Name = "Download" }).ClickAsync(); | ||||||
|  |         });`);
 | ||||||
|     expect.soft(sources.get('C#').text).toContain(` |     expect.soft(sources.get('C#').text).toContain(` | ||||||
|         var download1 = await page.RunAndWaitForDownloadAsync(async () => |         var download1 = await page.RunAndWaitForDownloadAsync(async () => | ||||||
|         { |         { | ||||||
|  | @ -299,13 +313,13 @@ test.describe('cli codegen', () => { | ||||||
|     await page.get_by_role("button", name="click me").click()`);
 |     await page.get_by_role("button", name="click me").click()`);
 | ||||||
| 
 | 
 | ||||||
|     expect.soft(sources.get('C#').text).toContain(` |     expect.soft(sources.get('C#').text).toContain(` | ||||||
|         void page_Dialog1_EventHandler(object sender, IDialog dialog) |         void page_Dialog_EventHandler(object sender, IDialog dialog) | ||||||
|         { |         { | ||||||
|             Console.WriteLine($\"Dialog message: {dialog.Message}\"); |             Console.WriteLine($\"Dialog message: {dialog.Message}\"); | ||||||
|             dialog.DismissAsync(); |             dialog.DismissAsync(); | ||||||
|             page.Dialog -= page_Dialog1_EventHandler; |             page.Dialog -= page_Dialog_EventHandler; | ||||||
|         } |         } | ||||||
|         page.Dialog += page_Dialog1_EventHandler; |         page.Dialog += page_Dialog_EventHandler; | ||||||
|         await page.GetByRole(AriaRole.Button, new() { Name = "click me" }).ClickAsync();`);
 |         await page.GetByRole(AriaRole.Button, new() { Name = "click me" }).ClickAsync();`);
 | ||||||
| 
 | 
 | ||||||
|   }); |   }); | ||||||
|  | @ -348,12 +362,11 @@ test.describe('cli codegen', () => { | ||||||
|         await page1.GotoAsync("about:blank?foo");`);
 |         await page1.GotoAsync("about:blank?foo");`);
 | ||||||
|     } else { |     } else { | ||||||
|       expect(sources.get('JavaScript').text).toContain(` |       expect(sources.get('JavaScript').text).toContain(` | ||||||
|   const [page1] = await Promise.all([ |   const page1Promise = page.waitForEvent('popup'); | ||||||
|     page.waitForEvent('popup'), |   await page.getByRole('link', { name: 'link' }).click({ | ||||||
|     page.getByRole('link', { name: 'link' }).click({ |  | ||||||
|     modifiers: ['${platform === 'darwin' ? 'Meta' : 'Control'}'] |     modifiers: ['${platform === 'darwin' ? 'Meta' : 'Control'}'] | ||||||
|     }) |   }); | ||||||
|   ]);`);
 |   const page1 = await page1Promise;`);
 | ||||||
|     } |     } | ||||||
|   }); |   }); | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue