Fixes to breaking tests #6074

DonJayamanne · 2021-06-02T20:00:10Z

codecov-commenter · 2021-06-02T20:31:34Z

Codecov Report

Merging #6074 (d74a598) into main (ac66cae) will decrease coverage by 1%.
The diff coverage is 76%.

@@          Coverage Diff           @@
##            main   #6074    +/-   ##
======================================
- Coverage     71%     70%    -2%     
======================================
  Files        398     398            
  Lines      26820   26847    +27     
  Branches    3942    3950     +8     
======================================
- Hits       19268   18805   -463     
- Misses      5927    6450   +523     
+ Partials    1625    1592    -33

Impacted Files	Coverage Δ
src/client/common/application/notebook.ts	`84% <0%> (-4%)`	⬇️
src/client/datascience/notebook/helpers/helpers.ts	`73% <ø> (+1%)`	⬆️
src/client/datascience/types.ts	`100% <ø> (ø)`
...rc/client/datascience/commands/notebookCommands.ts	`55% <66%> (-15%)`	⬇️
src/client/datascience/notebook/notebookEditor.ts	`38% <73%> (+2%)`	⬆️
...t/datascience/notebook/creation/notebookCreator.ts	`92% <100%> (+1%)`	⬆️
...t/datascience/notebook/vscodeNotebookController.ts	`77% <100%> (+<1%)`	⬆️
.../client/datascience/plotting/plotViewerProvider.ts	`38% <0%> (-48%)`	⬇️
.../datascience/plotting/plotViewerMessageListener.ts	`15% <0%> (-47%)`	⬇️
src/client/datascience/webviewExtensibility.ts	`32% <0%> (-40%)`	⬇️
... and 50 more

DonJayamanne · 2021-06-03T16:11:42Z

src/client/datascience/commands/notebookCommands.ts

-                this.commandManager.executeCommand(cmd).then(noop, noop);
-            }
+        if (this.notebookEditorProvider.activeEditor?.toggleOutput) {
+            this.notebookEditorProvider.activeEditor.toggleOutput();


I think its cleaner to have it outside this class (that's what we do for other code).

DonJayamanne · 2021-06-03T16:12:35Z

src/test/datascience/notebook/creation/notebookCreation.vscode.test.ts

@@ -44,7 +44,13 @@ suite('DataScience - VSCode Notebook - (Creation Integration)', function () {
        sinon.restore();
        await closeNotebooksAndCleanUpAfterTests(disposables);
    });
-    teardown(async () => closeNotebooksAndCleanUpAfterTests(disposables));


This is a duplicate.

DonJayamanne · 2021-06-03T17:52:53Z

src/client/common/application/notebook.ts

            return;
        }
        try {
+            console.error(`window.visibleNotebookEditors.length = ${window.visibleNotebookEditors.length}`);


To be removed.

😆 I've got a PR with all the same logging in. Does this PR have a fix for the creation issue? I wasn't seeing that offhand. Checking since I was looking at that as well.

Looks like more tests are failing because of this, i just found that out (my tests were failing because of this as well)
I thought my tests was failing due to some popup (but that wasn't the case), the notebook wasn't getting created.

I had a PR here that does fix the verify cells test, but I think it's a hack.
https://github.com/microsoft/vscode-jupyter/pull/6069/files
I explicitly check for one visible document being open, and then I force it active. But I'm not sure this is a good fix as I don't know yet what was making it inactive.

At least in that case the notebook was getting created, but it's getting set off of active by something. Doesn't happen locally.

Damn it, will need to debug this in VSCode and notify VSCode where the actual bug is

IanMatthewHuff · 2021-06-03T18:00:13Z

src/client/common/application/notebook.ts

@@ -18,7 +18,9 @@ import {
    window,
    workspace
 } from 'vscode';
+import { IS_CI_SERVER } from '../../../test/ciConstants';


I don't believe that a constant like this would be an issue either way. But I've seen issues before where we referenced a check from src/test in product code and it caused an issue by causing the test code to initialize. I believe that isCI from src/client/common/constants is what we would want here. It's the same thing, but not referencing into test source from product code.

Looking at the code I see we do this in other spots, so if you want to leave this for now I can file a code-heath issue to clean up all references of this in a later checkin.

I'll remove this

Actually it was already removed.

DonJayamanne · 2021-06-03T20:20:55Z

src/client/datascience/notebook/creation/notebookCreator.ts

@@ -45,6 +48,7 @@ export class NotebookCreator {
            label: JVSC_EXTENSION_DisplayName
        });
        const placeHolder = DataScience.placeHolderToSelectOptionForNotebookCreation();
+        traceInfoIf(IS_CI_SERVER, `Display quick pick for creation of notebooks ${items.length}`);


Damn it, will remove this in another PR.

@DonJayamanne it's fine I think that I need to remove them all as a code-health issue. I don't believe it matters now. I'll file an issue.

* Fix test failures resulting from VSCodes Notebook Start page (#6111) * Disable kernel auto startup in untrusted workspace (#6088) * Disable kernel auto startup in untrusted workspace * Fixes * Misc * oops * misc * Fixes to breaking tests (#6074) Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com>

* release candidate (#6059) * release candidate * Memorial Day API changes (#6056) * update changelog Co-authored-by: Ian Huff <ianhu@microsoft.com> * Memorial Day API changes (#6056) * Send only error type in reason (#6067) * Fallback for sys.prefix not being returned by Python extension (#6053) * Fixes to toggling output (#6068) * Better telemetry when we don't find a matching kernel (#6060) * Better data when fail to find kernel connection (#6070) * change version and API port (#6085) * change version to 2021.6.x * match packagejson and changelog versions * 6/2 API Changes (#6089) Co-authored-by: Ian Huff <ianhu@microsoft.com> * Port #6025 to release (#6103) * Remove our Run Above/Below commands (#6026) * Update changelog * Fix pervasive test issue with editor properties (#6100) (#6106) * update version to 2021.6.99 to be able to filter (#6122) * update version to 2021.6.99 to be able to filter update changelog * update package lock and VSCode api * don * remove api changes in the code * Port Don's fixes (#6115) * Fix test failures resulting from VSCodes Notebook Start page (#6111) * Disable kernel auto startup in untrusted workspace (#6088) * Disable kernel auto startup in untrusted workspace * Fixes * Misc * oops * misc * Fixes to breaking tests (#6074) Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> * Breaking changes ports to release (2 commits) (#6140) * add component governance file (#6166) * add component governance file * remove npm components * Cherry pick changes from main branch into release (#6174) * final update (#6176) * final update * update changelog * update verion, engine revert change on gulpfile * merge main * revert engine to 1.57-insider * delete news files that are on the changelog * disable insiders build Co-authored-by: Ian Huff <ianhu@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>

* release candidate (#6059) * release candidate * Memorial Day API changes (#6056) * update changelog Co-authored-by: Ian Huff <ianhu@microsoft.com> * Memorial Day API changes (#6056) * Send only error type in reason (#6067) * Fallback for sys.prefix not being returned by Python extension (#6053) * Fixes to toggling output (#6068) * Better telemetry when we don't find a matching kernel (#6060) * Better data when fail to find kernel connection (#6070) * change version and API port (#6085) * change version to 2021.6.x * match packagejson and changelog versions * 6/2 API Changes (#6089) Co-authored-by: Ian Huff <ianhu@microsoft.com> * Port #6025 to release (#6103) * Remove our Run Above/Below commands (#6026) * Update changelog * Fix pervasive test issue with editor properties (#6100) (#6106) * update version to 2021.6.99 to be able to filter (#6122) * update version to 2021.6.99 to be able to filter update changelog * update package lock and VSCode api * don * remove api changes in the code * Port Don's fixes (#6115) * Fix test failures resulting from VSCodes Notebook Start page (#6111) * Disable kernel auto startup in untrusted workspace (#6088) * Disable kernel auto startup in untrusted workspace * Fixes * Misc * oops * misc * Fixes to breaking tests (#6074) Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> * Breaking changes ports to release (2 commits) (#6140) * add component governance file (#6166) * add component governance file * remove npm components * Cherry pick changes from main branch into release (#6174) * final update (#6176) * final update * update changelog * add missing thanks to changelog (#6197) * Skip uploading vsix to azure blob store (#6204) * publish release * publish release * remove last 2 numbers from release (#6219) * publish release * fix conflicts * update engine to 1.58.0-insider * return engine version to 1.57.0-insider * Do not activate Python before opening nb (#6201) (#6230) * publish release Co-authored-by: Ian Huff <ianhu@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>

* release candidate (#6059) * release candidate * Memorial Day API changes (#6056) * update changelog Co-authored-by: Ian Huff <ianhu@microsoft.com> * Memorial Day API changes (#6056) * Send only error type in reason (#6067) * Fallback for sys.prefix not being returned by Python extension (#6053) * Fixes to toggling output (#6068) * Better telemetry when we don't find a matching kernel (#6060) * Better data when fail to find kernel connection (#6070) * change version and API port (#6085) * change version to 2021.6.x * match packagejson and changelog versions * 6/2 API Changes (#6089) Co-authored-by: Ian Huff <ianhu@microsoft.com> * Port #6025 to release (#6103) * Remove our Run Above/Below commands (#6026) * Update changelog * Fix pervasive test issue with editor properties (#6100) (#6106) * update version to 2021.6.99 to be able to filter (#6122) * update version to 2021.6.99 to be able to filter update changelog * update package lock and VSCode api * don * remove api changes in the code * Port Don's fixes (#6115) * Fix test failures resulting from VSCodes Notebook Start page (#6111) * Disable kernel auto startup in untrusted workspace (#6088) * Disable kernel auto startup in untrusted workspace * Fixes * Misc * oops * misc * Fixes to breaking tests (#6074) Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> * Breaking changes ports to release (2 commits) (#6140) * add component governance file (#6166) * add component governance file * remove npm components * Cherry pick changes from main branch into release (#6174) * final update (#6176) * final update * update changelog * add missing thanks to changelog (#6197) * Skip uploading vsix to azure blob store (#6204) * publish release * publish release * remove last 2 numbers from release (#6219) * publish release * Do not activate Python before opening nb (#6201) (#6230) * publish release * Port LiveKernelModel fix to release for point release (#6264) * Port keybinding updates to release (#6268) * Contribute shift+enter, ctrl+enter, L, shift+L (#6205) * Ctrl+Enter should put cell into command mode after executing (#6231) * Update CHANGELOG and remove news * port test fix (#6275) * Fix Restarting kernel... test (#6267) * wait for the restart command * add news file * update changelog * Fix native notebook interrupt toolbar (#6280) (#6282) * update changelog (#6284) * -update version -update changelog * leave version as it was * Respect jupyter.enableKeyboardShortcuts setting and enable ctrl+enter in command mode (#6293) * publish release * fix gulpfile * undo change Co-authored-by: Ian Huff <ianhu@microsoft.com> Co-authored-by: Don Jayamanne <don.jayamanne@yahoo.com> Co-authored-by: Don Jayamanne <don.jayamanne@outlook.com> Co-authored-by: Joyce Er <joyceerhuiling@gmail.com>

DonJayamanne force-pushed the fixNewNotebook branch 2 times, most recently from b0332bf to fbf6d95 Compare June 3, 2021 14:41

DonJayamanne added 8 commits June 3, 2021 07:48

logging

fb7fad7

Logging

c35a2b5

Clear

b29cb54

clear

04f8136

oops

cace3c3

oops

a557ff5

logging

ad48f15

misc

fa3feda

DonJayamanne force-pushed the fixNewNotebook branch from fbf6d95 to fa3feda Compare June 3, 2021 14:49

DonJayamanne added 7 commits June 3, 2021 07:50

oops

584ea3c

Misc

75bfc8d

Fix tests

92eac4e

logging

b12d67d

More logging

74581f4

Fix formatting

25a2203

Misc

11758ca

DonJayamanne changed the title ~~WIP~~ Fixes to breaking tests Jun 3, 2021

DonJayamanne commented Jun 3, 2021

View reviewed changes

Misc

ae6cfa5

DonJayamanne commented Jun 3, 2021

View reviewed changes

DonJayamanne added 5 commits June 3, 2021 09:24

Fix linter

4ee7e63

Add logging

05674c4

Revert

7481291

logging

2458407

More logging

89a4bfb

DonJayamanne marked this pull request as ready for review June 3, 2021 17:52

DonJayamanne requested a review from a team as a code owner June 3, 2021 17:52

DonJayamanne commented Jun 3, 2021

View reviewed changes

IanMatthewHuff reviewed Jun 3, 2021

View reviewed changes

DonJayamanne added 3 commits June 3, 2021 11:12

Revert

29c5492

Disable workspace trust in tests

43bd747

Prevent unnecessary errors

d74a598

DonJayamanne requested review from IanMatthewHuff, rchiodo, DavidKutu and joyceerhl June 3, 2021 18:31

DavidKutu approved these changes Jun 3, 2021

View reviewed changes

DonJayamanne merged commit e3ea1de into main Jun 3, 2021

DonJayamanne deleted the fixNewNotebook branch June 3, 2021 20:12

DonJayamanne commented Jun 3, 2021

View reviewed changes

DavidKutu pushed a commit that referenced this pull request Jun 3, 2021

Fixes to breaking tests (#6074)

5b1b0ec

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes to breaking tests #6074

Fixes to breaking tests #6074

DonJayamanne commented Jun 2, 2021 •

edited

Loading

codecov-commenter commented Jun 2, 2021 •

edited

Loading

DonJayamanne Jun 3, 2021

DonJayamanne Jun 3, 2021 •

edited

Loading

DonJayamanne Jun 3, 2021

IanMatthewHuff Jun 3, 2021

DonJayamanne Jun 3, 2021

IanMatthewHuff Jun 3, 2021

IanMatthewHuff Jun 3, 2021

DonJayamanne Jun 3, 2021

IanMatthewHuff Jun 3, 2021

DonJayamanne Jun 3, 2021

DonJayamanne Jun 3, 2021

DonJayamanne Jun 3, 2021

IanMatthewHuff Jun 3, 2021

Fixes to breaking tests #6074

Fixes to breaking tests #6074

Conversation

DonJayamanne commented Jun 2, 2021 • edited Loading

codecov-commenter commented Jun 2, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

DonJayamanne Jun 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DonJayamanne commented Jun 2, 2021 •

edited

Loading

codecov-commenter commented Jun 2, 2021 •

edited

Loading

DonJayamanne Jun 3, 2021 •

edited

Loading