Treat Native notebook tests as VS Code tests #14282
Merged
Conversation
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
DonJayamanne changed the title 
Codecov Report
@@ Coverage Diff @@ ## main #14282 +/- ## ========================================== + Coverage 59.75% 59.88% +0.13% ========================================== Files 697 709 +12 Lines 38649 39334 +685 Branches 5577 5698 +121 ========================================== + Hits 23094 23555 +461 - Misses 14364 14539 +175 - Partials 1191 1240 +49
Continue to review full report at Codecov.
|
DonJayamanne requested review from DavidKutu, IanMatthewHuff, greazer, joyceerhl and rchiodo 
Codecov Report
@@ Coverage Diff @@ ## main #14282 +/- ## ======================================= Coverage ? 59.89% ======================================= Files ? 709 Lines ? 39334 Branches ? 5698 ======================================= Hits ? 23558 Misses ? 14535 Partials ? 1241
Continue to review full report at Codecov.
|
DonJayamanne commented Oct 6, 2020
| await this.dequeue(); | ||
| } | ||
| this._result.resolve(this.cell.metadata.runState); | ||
| await this.completedDurToCancellation(); |
There was a problem hiding this comment.
Due to changes in API, we need to ensure cell status is updated.
DonJayamanne commented Oct 6, 2020
| | ||
| // Confirm it is installed. | ||
| sinon.stub(installer, 'install').callsFake(async function (product: Product) { | ||
| const showInformationMessage = sinon.stub(installer, 'install').callsFake(async function (product: Product) { |
There was a problem hiding this comment.
We need to revert the stubs in each test.
DonJayamanne commented Oct 6, 2020
| const appShell = api.serviceContainer.get<IApplicationShell>(IApplicationShell); | ||
| const showInformationMessage = sinon | ||
| .stub(appShell, 'showInformationMessage') | ||
| .callsFake(function (message: string) { |
There was a problem hiding this comment.
Better & easier test, than updating the settings
There was a problem hiding this comment.
Also can be used to confirm prompt is displayed.
DonJayamanne commented Oct 6, 2020
| let testIPynb: Uri; | ||
| const disposables: IDisposable[] = []; | ||
| suiteSetup(async function () { | ||
| return this.skip(); |
There was a problem hiding this comment.
THis is the only other native notebook test that is failing.
Trusted notebooks don't work, hence disabled @IanMatthewHuff /cc
rchiodo approved these changes Oct 6, 2020
DavidKutu reviewed Oct 6, 2020
| } | ||
| }); | ||
| test('Restarting kernel will cancel cell execution & we can re-run a cell', async () => { | ||
| test('Restarting kernel will cancel cell execution & we can re-run a cellxxx', async () => { |
DavidKutu approved these changes Oct 6, 2020
joyceerhl approved these changes Oct 6, 2020
| Kudos, SonarCloud Quality Gate passed!
|
DonJayamanne added a commit that referenced this pull request Oct 30, 2020
* Remove cell index property and use build in prop (#14239) * Update other cells in cell execution (#14240) * Tests for prompting to install missing ipykernel (#14266) * Treat Native notebook tests as VS Code tests (#14282) * Fixes to blowing away of kernel info & not using right startup info (… … * Default cell language for native notebooks (#14314) * Ignore formatting in ipynb when dealing with trust (#14333) * Fixes to trust service (#14352) * Change `IPython kernel` to `Jupyter kernel` (#14375) * Trust for native notebooks (#14353)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
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.
Basically rename ds.test.ts to native.vscode.test.ts
Two of the native notebook tests are failing (fixed one & disabled the other).
Make it easier to migrate
ds.test.tsto.vscode.test.ts.Both are tests running in VSCode, hecne have a common way to do it.