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

switch to esmock, re #221 #225

Merged
merged 5 commits into from Jul 19, 2022
Merged

switch to esmock, re #221 #225

merged 5 commits into from Jul 19, 2022

Conversation

iambumblehead
Copy link
Contributor

@iambumblehead iambumblehead commented Jul 17, 2022

Fixes #221

I hope the changes here are OK and wanted to mention two things. One thing, some tests, expected to fail, were passing in my local environment. I found this this recent commit which indicated to me that the passing tests might be OK -- not sure. The tests in question are found in test/notifier.js.

The other thing is, esmock-imported modules are each using their own state, so tests could be updated in the future to run concurrently rather than sequentially (here).

@sindresorhus
Copy link
Member

CI is failing

@iambumblehead
Copy link
Contributor Author

Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'clear-module' imported from /home/runner/work/update-notifier/update-notifier/test/fs-error.js

I'll resolve it in a moment

@iambumblehead
Copy link
Contributor Author

its ready

process.env.NO_UPDATE_NOTIFIER = '1';
const notifier = updateNotifier(generateSettings());
t.is(notifier.config, undefined);
});

test('don\'t initialize configStore when --no-update-notifier is set', t => {
test('don\'t initialize configStore when --no-update-notifier is set', async t => {
const updateNotifier = await esmock('../index.js', null, {'is-ci': false});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const updateNotifier = await esmock('../index.js', null, {'is-ci': false});
const updateNotifier = await esmock('../index.js', undefined, {'is-ci': false});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this added to the commit pushed below

@sindresorhus sindresorhus merged commit 3046d0f into yeoman:main Jul 19, 2022
@sindresorhus
Copy link
Member

Thanks :)

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.

Switch to esmock
3 participants