Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Main VariableView test code #4400

Merged
merged 22 commits into from
Jan 16, 2021
Merged

Conversation

IanMatthewHuff
Copy link
Member

For #4355

  • 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!).
  • Appropriate comments and documentation strings in the code.
  • Has sufficient logging.
  • Has telemetry for enhancements.
  • 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).

@IanMatthewHuff
Copy link
Member Author

Note: I'll have to merge up with the eslint change, and fix the xvfb action. But this is a full example of the variable view tests. Have a question or two on this, so would like to get the PR out while fixing the github action.

@@ -207,6 +207,7 @@
"VSC_FORCE_REAL_JUPYTER": "true", // Enalbe tests that require Jupyter.
"VSC_JUPYTER_CI_RUN_NON_PYTHON_NB_TEST": "", // Initialize this to run tests again Julia & other kernels.
"VSC_JUPYTER_RUN_NB_TEST": "true", // Initialize this to run notebook tests (must be using VSC Insiders).
"VSC_JUPYTER_WEBVIEW_TEST_MIDDLEWARE": "true", // Initialize to create the webview test middleware
Copy link
Member Author

@IanMatthewHuff IanMatthewHuff Jan 15, 2021

Choose a reason for hiding this comment

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

Add an env var to force on the test middle ware, needed for the middleware VariablesComplete message, and handy to be able to turn on quick just when running VS Code from debugger.

}

// For our target object wait for a specific message to come in from onMessage function
public async waitForMessage(message: string, options?: WaitForMessageOptions): Promise<void> {
Copy link
Member Author

Choose a reason for hiding this comment

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

The waitForMessage implementation here is very close to the existing one in mountedWebView.ts. Figured that interface already existed and was doing the same basic thing so just reused it here with minor changes for not being part of a mountedWebView.

@IanMatthewHuff IanMatthewHuff marked this pull request as ready for review January 15, 2021 01:24
@IanMatthewHuff IanMatthewHuff requested a review from a team as a code owner January 15, 2021 01:24
suiteTeardown(() => closeNotebooksAndCleanUpAfterTests(disposables));

// Test showing the basic variable view with a value or two
test('Can show VariableView', async function () {
Copy link
Member Author

Choose a reason for hiding this comment

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

More tests in the queue. But wanted to get approval here and get one basic test running on CI before adding more.

@codecov-io
Copy link

codecov-io commented Jan 15, 2021

Codecov Report

Merging #4400 (5d51678) into main (e8f311a) will decrease coverage by 9%.
The diff coverage is 40%.

@@           Coverage Diff           @@
##            main   #4400     +/-   ##
=======================================
- Coverage     75%     65%    -10%     
=======================================
  Files        392     392             
  Lines      25627   25671     +44     
  Branches    3661    3669      +8     
=======================================
- Hits       19371   16887   -2484     
- Misses      4753    7465   +2712     
+ Partials    1503    1319    -184     
Impacted Files Coverage Δ
.../datascience/interactive-common/synchronization.ts 44% <ø> (-40%) ⬇️
src/client/datascience/variablesView/types.ts 100% <ø> (ø)
.../datascience/variablesView/variableViewProvider.ts 64% <18%> (-20%) ⬇️
src/client/datascience/webviews/webviewHost.ts 76% <33%> (-12%) ⬇️
src/client/common/utils/decorators.ts 76% <62%> (-2%) ⬇️
src/client/common/application/webviews/webview.ts 67% <100%> (+<1%) ⬆️
...ience/interactive-common/interactiveWindowTypes.ts 100% <100%> (ø)
src/client/datascience/liveshare/serviceProxy.ts 5% <0%> (-83%) ⬇️
...ent/datascience/jupyter/liveshare/responseQueue.ts 24% <0%> (-72%) ⬇️
...rc/client/datascience/jupyter/debuggerVariables.ts 16% <0%> (-68%) ⬇️
... and 98 more

@IanMatthewHuff IanMatthewHuff merged commit 3236ec1 into main Jan 16, 2021
@IanMatthewHuff IanMatthewHuff deleted the dev/ianhu/variableViewTestFinal branch January 16, 2021 00:28
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.

5 participants