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.

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

Enable esModuleInterop to ease the transition to ES modules #2057

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Page-
Copy link
Contributor

@Page- Page- commented Nov 11, 2022

Change-type: patch

If this is a regression, consider adding it to #1898!

Description

Please include a summary of the change and which issue was fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Include at least one commit in your PR that marks the change-type. This can be either specified through a Change-type footer, or by adding the change-type as a prefix to the commit, i.e. minor: Add some new feature. This is so the PR can be automatically versioned and a changelog generated for it by using versionist. Check out semver for a detailed explanation of the different possible change-type values.

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

@Page- Page- marked this pull request as ready for review November 22, 2022 12:51
@Page- Page- requested review from pipex, cywang117 and kb2ma and removed request for pipex November 22, 2022 12:51
@pipex
Copy link
Contributor

pipex commented Nov 22, 2022

No idea why the tests are failing here, I could not reproduce the following locally

 balena-supervisor-sut-1  |   1) lib/firewall
balena-supervisor-sut-1  |        Basic On/Off operation
balena-supervisor-sut-1  |          should confirm the `changed` event is handled:
balena-supervisor-sut-1  |      TypeError: Descriptor for property spawn is non-configurable and non-writable
balena-supervisor-sut-1  |       at assertValidPropertyDescriptor (node_modules/sinon/lib/sinon/stub.js:158:15)
balena-supervisor-sut-1  |       at Function.stub (node_modules/sinon/lib/sinon/stub.js:85:5)
balena-supervisor-sut-1  |       at stub (node_modules/sinon/lib/sinon/sandbox.js:388:37)

@Page-
Copy link
Contributor Author

Page- commented Nov 22, 2022

@pipex the issue is that esModuleInterop enforces that a module's exports cannot be modified (since in real ESM they cannot be) and stub(childProcess, 'spawn').callsFake(...) is attempting to modify the child_process exports

@cywang117
Copy link
Contributor

cywang117 commented Nov 22, 2022

Does that explain why the test error can't be produced locally though @Page- ?

Looking online, there doesn't seem to be any clear solution other than wrapping the thing that calls spawn in a function, and stubbing that wrapper function.

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.

3 participants