Skip to content
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.

Sign up for GitHub

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

Jump to bottom

refactor: move util/fs under util/gitfs #6618

Merged
merged 3 commits into from Jun 28, 2020
Merged

refactor: move util/fs under util/gitfs #6618

merged 3 commits into from Jun 28, 2020

Conversation

rarkins
Copy link
Collaborator

@rarkins rarkins commented Jun 28, 2020

Part of #6617

@rarkins rarkins requested a review from zharinov June 28, 2020 07:16
@rarkins
Copy link
Collaborator Author

rarkins commented Jun 28, 2020

@zharinov I can't work out why the readLocalFile mocked function is being passed undefined filenames. Can you take a look? Note: the npm manager was the only code where I needed to add any extra mocking after doing this moving/renaming.

@rarkins
Copy link
Collaborator Author

rarkins commented Jun 28, 2020

Before adding the jest.mock I got this:

image

viceice
viceice reviewed Jun 28, 2020
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

It's because the way typescript handle imports. If you do import * as xxx from 'xxx' xxx will only have readonly properties.

lib/manager/npm/post-update/node-version.spec.ts
const fs = mocked(fs_);
jest.mock('../../../util/gitfs');

const gitfs = mocked(_gitfs);
Copy link
Member

Choose a reason for hiding this comment

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

Should be imported from test/util

@viceice
Copy link
Member

viceice commented Jun 28, 2020

It's a Babel issue too. For tests we use Babel to transpile ts to js. So maybe Babel makes the exports object readonly properties.

@rarkins
Copy link
Collaborator Author

rarkins commented Jun 28, 2020

Found some useful explanations here: https://stackoverflow.com/questions/53162001/typeerror-during-jests-spyon-cannot-set-property-getrequest-of-object-which

@rarkins rarkins requested a review from viceice June 28, 2020 07:48
viceice
viceice reviewed Jun 28, 2020
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

Just some test changes, we should import gitfs from test/util

lib/util/gitfs/index.ts Show resolved Hide resolved
lib/workers/repository/extract/manager-files.spec.ts Show resolved Hide resolved
@rarkins rarkins added this to In progress in Performance Improvement Jun 28, 2020
@rarkins rarkins merged commit ec15985 into master Jun 28, 2020
Performance Improvement automation moved this from In progress to Done Jun 28, 2020
@rarkins rarkins deleted the refactor/gitfs branch June 28, 2020 09:57
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 21.19.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@viceice viceice viceice left review comments

@zharinov zharinov Awaiting requested review from zharinov

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Performance Improvement
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rarkins @viceice @renovate-release