Skip to content

Bypass updates - #306

Merged
tschaub merged 3 commits into
masterfrom
bypass-updates
Aug 9, 2020
Merged

Bypass updates#306
tschaub merged 3 commits into
masterfrom
bypass-updates

Conversation

@tschaub

@tschaub tschaub commented Aug 9, 2020

Copy link
Copy Markdown
Owner

This is a follow-up to #304.

@tschaub
tschaub merged commit f004073 into master Aug 9, 2020
@tschaub
tschaub deleted the bypass-updates branch August 9, 2020 22:19
@3cp

3cp commented Aug 9, 2020

Copy link
Copy Markdown
Collaborator

Instead of playing _mockedBinding, it's probably cheaper and less code if we use a boolean flag on realBinding to temporarily disable mocking.

@tschaub

tschaub commented Aug 10, 2020

Copy link
Copy Markdown
Owner Author

It looks like there will be more to do to get bypass fully working. fs.createWriteStream doesn't work with the current implementation, and process.cwd() and process.chdir() will produce unexpected results.

@3cp

3cp commented Aug 10, 2020

Copy link
Copy Markdown
Collaborator

The patches for cwd/chdir/createWriteStream/binding should all check the same _mockedBinding (or a flag) on realBinding. This also means we only need to patch them once without real restore.

The restore (not just for bypass) only needs to remove _mockedBinding (and the flag).

I will create a draft.

@3cp

3cp commented Aug 10, 2020

Copy link
Copy Markdown
Collaborator

Could you merge #303 first?

3cp added a commit that referenced this pull request Aug 13, 2020
All the patches now check same realBinding._mockedBinding. This should
fix the edge cases around the new feature bypass().

follows up #306
3cp added a commit that referenced this pull request Aug 15, 2020
All the patches now check same realBinding._mockedBinding. This should
fix the edge cases around the new feature bypass().

follows up #306
3cp added a commit that referenced this pull request Aug 15, 2020
All the patches now check same realBinding._mockedBinding. This should
fix the edge cases around the new feature bypass().

follows up #306
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants