-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Treat Native notebook tests as VS Code tests #14282
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
Conversation
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.
|
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.
|
@@ -156,7 +160,7 @@ export class CellExecution { | |||
if (!this.started) { | |||
await this.dequeue(); | |||
} | |||
this._result.resolve(this.cell.metadata.runState); | |||
await this.completedDurToCancellation(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to changes in API, we need to ensure cell status is updated.
@@ -72,7 +73,7 @@ suite('DataScience Install IPyKernel (slow) (install)', () => { | |||
const installed = createDeferred(); | |||
|
|||
// 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to revert the stubs in each test.
const appShell = api.serviceContainer.get<IApplicationShell>(IApplicationShell); | ||
const showInformationMessage = sinon | ||
.stub(appShell, 'showInformationMessage') | ||
.callsFake(function (message: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better & easier test, than updating the settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also can be used to confirm prompt is displayed.
@@ -37,6 +37,7 @@ suite('DataScience - VSCode Notebook - (Trust)', function () { | |||
let testIPynb: Uri; | |||
const disposables: IDisposable[] = []; | |||
suiteSetup(async function () { | |||
return this.skip(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THis is the only other native notebook test that is failing.
Trusted notebooks don't work, hence disabled @IanMatthewHuff /cc
@@ -115,10 +103,23 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)', | |||
assertVSCCellHasErrors(cell); | |||
} | |||
}); | |||
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 () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the celxxx meant to be there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
damn
Kudos, SonarCloud Quality Gate passed!
|
* 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)
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.ts
to.vscode.test.ts
.Both are tests running in VSCode, hecne have a common way to do it.