Skip to content

chore(vscode): starts testing the actual contents of the lineage view#4865

Merged
benfdking merged 1 commit intomainfrom
enabling_lineage_tests
Jul 1, 2025
Merged

chore(vscode): starts testing the actual contents of the lineage view#4865
benfdking merged 1 commit intomainfrom
enabling_lineage_tests

Conversation

@benfdking
Copy link
Copy Markdown
Contributor

@benfdking benfdking commented Jul 1, 2025

  • starts testing the contents of the lineage view

- 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
@benfdking benfdking force-pushed the enabling_lineage_tests branch from 5399797 to 1f0d016 Compare July 1, 2025 16:45
@benfdking benfdking changed the title chore: speed up tests when running locally chore(vscode): starts testing the actual contents of the lineage view Jul 1, 2025
@benfdking benfdking requested a review from Copilot July 1, 2025 16:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 expect from 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_demographicsCount to rawDemographicsCount.
    let raw_demographicsCount = 0

Comment on lines +104 to +106
const contentFrame = iframe.contentFrame()
if (contentFrame) {
const activeFrame = contentFrame.locator('#active-frame').contentFrame()
Copy link

Copilot AI Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copilot uses AI. Check for mistakes.
const iframe = iframes.nth(i)
const contentFrame = iframe.contentFrame()
if (contentFrame) {
const activeFrame = contentFrame.locator('#active-frame').contentFrame()
Copy link

Copilot AI Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copilot uses AI. Check for mistakes.
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
}
}
}
}
Copy link

Copilot AI Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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')

Copilot uses AI. Check for mistakes.
@benfdking benfdking merged commit a3302e4 into main Jul 1, 2025
27 checks passed
@benfdking benfdking deleted the enabling_lineage_tests branch July 1, 2025 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants