chore(vscode): starts testing the actual contents of the lineage view#4865
Merged
chore(vscode): starts testing the actual contents of the lineage view#4865
Conversation
- when running the tests one by one in vscode for example - this skips the step of installation if already installed making the tests run faster
5399797 to
1f0d016
Compare
georgesittas
approved these changes
Jul 1, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
Adds assertions to verify the actual contents of the lineage view by iterating through nested iframes instead of relying on a single selector.
- Imported
expectfrom Playwright to enable assertions. - Replaced TODO’d selector waits with loops scanning all iframes for error and model text.
- Introduced three nearly identical iframe-scanning blocks for different test scenarios.
Comments suppressed due to low confidence (1)
vscode/extension/tests/broken_project.spec.ts:130
- [nitpick] Variable names should use consistent camelCase; consider renaming
raw_demographicsCounttorawDemographicsCount.
let raw_demographicsCount = 0
Comment on lines
+104
to
+106
| const contentFrame = iframe.contentFrame() | ||
| if (contentFrame) { | ||
| const activeFrame = contentFrame.locator('#active-frame').contentFrame() |
There was a problem hiding this comment.
You need to await the call to iframe.contentFrame() to get the actual Frame object before using it.
Suggested change
| const contentFrame = iframe.contentFrame() | |
| if (contentFrame) { | |
| const activeFrame = contentFrame.locator('#active-frame').contentFrame() | |
| const contentFrame = await iframe.contentFrame() | |
| if (contentFrame) { | |
| const activeFrame = await contentFrame.locator('#active-frame').contentFrame() |
| const iframe = iframes.nth(i) | ||
| const contentFrame = iframe.contentFrame() | ||
| if (contentFrame) { | ||
| const activeFrame = contentFrame.locator('#active-frame').contentFrame() |
There was a problem hiding this comment.
The call to .contentFrame() on the locator must also be awaited to retrieve the nested frame.
Suggested change
| const activeFrame = contentFrame.locator('#active-frame').contentFrame() | |
| const activeFrame = await contentFrame.locator('#active-frame').contentFrame() |
Comment on lines
+98
to
+119
| const iframes = page.locator('iframe') | ||
| const iframeCount = await iframes.count() | ||
| let errorCount = 0 | ||
|
|
||
| for (let i = 0; i < iframeCount; i++) { | ||
| const iframe = iframes.nth(i) | ||
| const contentFrame = iframe.contentFrame() | ||
| if (contentFrame) { | ||
| const activeFrame = contentFrame.locator('#active-frame').contentFrame() | ||
| if (activeFrame) { | ||
| try { | ||
| await activeFrame | ||
| .getByText('Error: Failed to load model') | ||
| .waitFor({ timeout: 1000 }) | ||
| errorCount++ | ||
| } catch { | ||
| // Continue to next iframe if this one doesn't have the error | ||
| continue | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
[nitpick] You repeat nearly identical iframe-scanning logic three times. Extract a helper function like countTextInLineageView(page, text) to reduce duplication.
Suggested change
| const iframes = page.locator('iframe') | |
| const iframeCount = await iframes.count() | |
| let errorCount = 0 | |
| for (let i = 0; i < iframeCount; i++) { | |
| const iframe = iframes.nth(i) | |
| const contentFrame = iframe.contentFrame() | |
| if (contentFrame) { | |
| const activeFrame = contentFrame.locator('#active-frame').contentFrame() | |
| if (activeFrame) { | |
| try { | |
| await activeFrame | |
| .getByText('Error: Failed to load model') | |
| .waitFor({ timeout: 1000 }) | |
| errorCount++ | |
| } catch { | |
| // Continue to next iframe if this one doesn't have the error | |
| continue | |
| } | |
| } | |
| } | |
| } | |
| const errorCount = await countTextInLineageView(page, 'Error: Failed to load model') |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.