Skip to content

Mock jupyter apis such that they can be faked to have tests run more reliably - #3853

Merged
Rich Chiodo (rchiodo) merged 5 commits into
masterfrom
rchiodo/jupyter_mock
Jan 3, 2019
Merged

Rich Chiodo (rchiodo) merged 5 commits into
masterfrom
rchiodo/jupyter_mock

Conversation

@rchiodo

Copy link
Copy Markdown

For #3556

  • Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
  • Title summarizes what is changing
  • Has a news entry file (remember to thank yourself!)
  • Unit tests & system/integration tests are added/updated
  • Test plan is updated as appropriate
  • package-lock.json has been regenerated by running npm install (if dependencies have changed)

// Do the same thing as dispose so that we regenerate
// all of our commands
return Promise.resolve(this.dispose());
this.dispose();

@rchiodo Rich Chiodo (rchiodo) Jan 3, 2019 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this [](start = 8, length = 4)

I think this might be incorrect. It should wait for the dispose. #Resolved

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oh actually the dispose() in this class doesn't return a promise, so this is correct.


In reply to: 245071855 [](ancestors = 245071855)

Comment thread src/client/datascience/jupyter/jupyterKernelSpec.ts Outdated
}

async function verifySimple(jupyterServer: INotebookServer | undefined, code: string, expectedValue: any): Promise<void> {
async function verifySimple(jupyterServer: INotebookServer | undefined, code: string, expectedValue: any) : Promise<void> {

@IanMatthewHuff Ian Huff (IanMatthewHuff) Jan 3, 2019 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[](start = 109, length = 1)

Not that I care too much, but this formatting seems funny. Did the linter add this? #WontFix

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not sure?


In reply to: 245085968 [](ancestors = 245085968)

@IanMatthewHuff Ian Huff (IanMatthewHuff) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

@rchiodo
Rich Chiodo (rchiodo) merged commit 71bf20d into master Jan 3, 2019
@rchiodo
Rich Chiodo (rchiodo) deleted the rchiodo/jupyter_mock branch January 3, 2019 18:35
@lock lock Bot locked as resolved and limited conversation to collaborators Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants