Skip to content

Add Python: Refresh TensorBoard command - #16068

Merged
Joyce Er (joyceerhl) merged 1 commit into
microsoft:mainfrom
joyceerhl:dev/joyceerhl/refresh-tensorboard
May 4, 2021
Merged

Add Python: Refresh TensorBoard command#16068
Joyce Er (joyceerhl) merged 1 commit into
microsoft:mainfrom
joyceerhl:dev/joyceerhl/refresh-tensorboard

Conversation

@joyceerhl

Copy link
Copy Markdown

For #16053

@karrtikr Kartik Raj (karrtikr) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No tests to update/add?

@joyceerhl

Copy link
Copy Markdown
Author

Kartik Raj (@karrtikr) I don't think it's possible to write a test to verify that this works because we can't inspect the iframed contents. It would be possible to confirm that the TensorBoardSession class refresh method is called when the command is executed, but that's about it.

@karrtikr Kartik Raj (karrtikr) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think it might be possible if you're able to stub window.createWebviewPanel and verify the contents of webviewPanel.webview.html. I leave it upto you.

@paulacamargo25 Paula (paulacamargo25) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@joyceerhl

Joyce Er (joyceerhl) commented Apr 29, 2021

Copy link
Copy Markdown
Author

Kartik Raj (@karrtikr) sorry no, I meant that it's not possible to verify the contents of the iframe that we create, because that has a cross-origin source (before rewriting, it's a localhost URL). The HTML will only give us the iframe element and the attributes, it won't give us what's actually rendered on the page.

@karrtikr

Copy link
Copy Markdown

Joyce Er (@joyceerhl) I see. Is it useful to atleast verify the the HTML in the test? If not, skipping tests is fine here I think.

@joyceerhl

Copy link
Copy Markdown
Author

Yeah the HTML is just going to be whatever we set. Not sure that's worth validating with a test.

@joyceerhl Joyce Er (joyceerhl) added skip package*.json package.json and package-lock.json don't both need updating skip tests Updates to tests unnecessary labels May 3, 2021
@joyceerhl
Joyce Er (joyceerhl) merged commit 812be6c into microsoft:main May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip package*.json package.json and package-lock.json don't both need updating skip tests Updates to tests unnecessary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants