Skip to content

Add basic functional tests for the high-level FileSystem class. - #9170

Merged
Eric Snow (ericsnowcurrently) merged 31 commits into
microsoft:masterfrom
ericsnowcurrently:fs-tests
Dec 31, 2019
Merged

Add basic functional tests for the high-level FileSystem class.#9170
Eric Snow (ericsnowcurrently) merged 31 commits into
microsoft:masterfrom
ericsnowcurrently:fs-tests

Conversation

@ericsnowcurrently

@ericsnowcurrently Eric Snow (ericsnowcurrently) commented Dec 17, 2019 •
edited
Loading

Copy link
Copy Markdown

(for #8995)

This adds back in high-level functional tests reverted in #8970, as well as adding some more. There is also a little bit of cleanup (group methods in FileSystem and IFileSystem).

@ericsnowcurrently Eric Snow (ericsnowcurrently) added the no-changelog No news entry required label Dec 17, 2019
@codecov-io

Codecov (codecov-io) commented Dec 17, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #9170 into master will decrease coverage by 0.39%.
The diff coverage is 30.09%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #9170     +/-   ##
=========================================
- Coverage   60.32%   59.92%   -0.4%     
=========================================
  Files         544      544             
  Lines       28594    28620     +26     
  Branches     4330     4328      -2     
=========================================
- Hits        17248    17151     -97     
- Misses      10415    10534    +119     
- Partials      931      935      +4
Impacted Files Coverage Δ
...rc/client/debugger/debugAdapter/serviceRegistry.ts 100% <ø> (ø) ⬆️
src/client/common/platform/types.ts 100% <100%> (ø) ⬆️
src/client/interpreter/locators/helpers.ts 67.39% <12.5%> (-3.07%) ⬇️
src/client/common/platform/fileSystem.ts 27.71% <30.1%> (-14.74%) ⬇️
src/client/testing/serviceRegistry.ts 49.09% <0%> (-47.28%) ⬇️
src/client/testing/codeLenses/main.ts 40% <0%> (-30%) ⬇️
src/client/datascience/jupyter/jupyterUtils.ts 64.7% <0%> (-29.42%) ⬇️
...t/datascience/jupyter/jupyterDataRateLimitError.ts 50% <0%> (-25%) ⬇️
src/client/formatters/serviceRegistry.ts 75% <0%> (-25%) ⬇️
...ience/jupyter/jupyterDebuggerRemoteNotSupported.ts 60% <0%> (-20%) ⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b8a603...679623c. Read the comment docs.

DonJayamanne
Don Jayamanne (DonJayamanne) requested changes Dec 18, 2019
View reviewed changes
Comment thread src/client/common/platform/fileSystem.ts
Comment thread src/client/common/platform/fileSystem.ts Outdated
Comment thread src/client/common/platform/fileSystem.ts Outdated
DonJayamanne
Don Jayamanne (DonJayamanne) requested changes Dec 23, 2019
View reviewed changes

@DonJayamanne Don Jayamanne (DonJayamanne) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I still disagree with what's going on in convertFileType.
It's not converting it like for like. I think the method is doing the wrong thing.
I think its best to add comments so its clear that we are ignoring the bitwise/flags values.
I'd expect the conversion to be a proper conversion. Instead this is merely returning a boolean state, isFile isDirectory, isSymbolicLink.

Please add the comments so the intention is clear.

@ericsnowcurrently

Eric Snow (ericsnowcurrently) commented Dec 30, 2019

Copy link
Copy Markdown
Author

fs.Stats objects only ever have one type, so I'm not sure what you mean by "not converting it like for like". That said, I don't mind adding a comment to make it clear. 😄

DonJayamanne
Don Jayamanne (DonJayamanne) requested changes Dec 31, 2019
View reviewed changes
Comment thread src/client/common/platform/fileSystem.ts
@ericsnowcurrently
Group the IFileSystem methods.
5f4ea1c
@ericsnowcurrently
Group FileSystem methods.
adc6b11
@ericsnowcurrently
Make the FS tests "functional".
7911537
@ericsnowcurrently
Drop IFileSystem.objectExists.
2069cef
@ericsnowcurrently
Re-organize tests.
64171ff
@ericsnowcurrently
Fix up the unit tests.
3917ce5
@ericsnowcurrently
Support numbers in chmod().
5ebb737
@ericsnowcurrently
Add a CleanupFixture and use it in FSFixture.
8db3cdb
@ericsnowcurrently
Unrevert a bunch of functional tests.
f0f565a
@ericsnowcurrently
Add some missing blank lines.
bf21a43
@ericsnowcurrently
Alias vscode's FileType and FileStat locally.
e28dea1
@ericsnowcurrently
Do not depend on IFileSystem in the old debug adapter.
a5f0fef
@ericsnowcurrently
Excise IPlatformService from the old debug adapter.
b441de6
@ericsnowcurrently
Fix the failing tests.
fb4a2d2
@ericsnowcurrently
Drop appendFileSync().
f32d5f3
@ericsnowcurrently
Add tests for appendFile().
5a41b81
@ericsnowcurrently
Drop unneeded TODO comments.
2b5e1b6
@ericsnowcurrently
Add tests for *Exists().
4f9060e
@ericsnowcurrently
Add tests for createReadStream().
7a11004
@ericsnowcurrently
Add tests for listdir().
1043045
@ericsnowcurrently
Add tests for lstat().
b3ea9ac
@ericsnowcurrently
Add tests for getRealPath().
522d297
@ericsnowcurrently
Eric Snow (ericsnowcurrently) merged commit ea02ed3 into microsoft:master Dec 31, 2019
This was referenced Jan 1, 2020
Closed
Merged
@karrtikr Kartik Raj (karrtikr) mentioned this pull request Jan 2, 2020
Closed
@lock lock Bot locked as resolved and limited conversation to collaborators Jan 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DonJayamanne Don Jayamanne (DonJayamanne) DonJayamanne approved these changes

Assignees

No one assigned

Labels

no-changelog No news entry required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ericsnowcurrently @codecov-io @DonJayamanne