Fixes to trust service #14352
Merged
Fixes to trust service #14352
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
requested review from
DavidKutu,
greazer,
IanMatthewHuff and
rchiodo
DonJayamanne
commented
Oct 9, 2020
| await trustService.trustNotebook(uri, contents); | ||
| assert.isTrue(await trustService.isNotebookTrusted(uri, contents), 'Notebook is not trusted'); | ||
| }); | ||
| test('Trusting a notebook (json saved with different formats)', async () => { |
There was a problem hiding this comment.
Something I should have added in the previous PR.
|
Kudos, SonarCloud Quality Gate passed!
|
rchiodo
approved these changes
Oct 9, 2020
joyceerhl
approved these changes
Oct 9, 2020
I ran the tests locally as well and it passed with the bug in t. |
Codecov Report
@@ Coverage Diff @@
## main #14352 +/- ##
==========================================
- Coverage 59.91% 59.43% -0.49%
==========================================
Files 709 716 +7
Lines 39334 40009 +675
Branches 5698 5799 +101
==========================================
+ Hits 23567 23779 +212
- Misses 14529 14970 +441
- Partials 1238 1260 +22
Continue to review full report at Codecov.
|
DavidKutu
approved these changes
Oct 9, 2020
| @@ -38,7 +38,13 @@ export class TrustService implements ITrustService { | |||
| this.computeDigest(this.getFormattedContents(notebookContents)) | |||
| ]); | |||
|
|
|||
| return this.digestStorage.containsDigest(uri, digest1) || this.digestStorage.containsDigest(uri, digest2); | |||
| // return this.digestStorage.containsDigest(uri, digest1) || this.digestStorage.containsDigest(uri, digest2); | |||
There was a problem hiding this comment.
Pull the old commented out code?
There was a problem hiding this comment.
Will remove in next PR, as CI is slow.
IanMatthewHuff
approved these changes
Oct 9, 2020
luabud
pushed a commit
to luabud/vscode-python
that referenced
this pull request
Oct 26, 2020
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
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.
I managed to break the trust service in a previous PR.
The tests didn't catch that, I wrote some real tests to test with VS Code (super fast test, run in 192ms, i.e. using VSC isnt an issue for such tests).
Basically trust was completely broken, nothing would get trusted.