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

Tests for config.js #14

Merged
merged 13 commits into from
May 13, 2020
Merged

Tests for config.js #14

merged 13 commits into from
May 13, 2020

Conversation

Sleavely
Copy link
Owner

@Sleavely Sleavely commented May 4, 2020

No description provided.

@Sleavely Sleavely marked this pull request as ready for review May 11, 2020 08:20
@Sleavely Sleavely requested a review from amoj May 11, 2020 08:20
Comment on lines +10 to +25
// Reset the config/context/state singletons with jest.resetModules().
// Unfortunately this seems to destroy mocks from the main scope
// so we have to redefine them for each test; I tried using
// `delete require.cache[require.resolve('./config')]`
// but Jest overrides the NodeJS require-mechanism and
// refuses to let you clear individual entries from the cache.
jest.resetModules()
jest.doMock('./utils/fs')
fs = require('./utils/fs')

// Emulate a config file
const configPath = './config.virtual'
fs.findUpwardsFile.mockResolvedValue(configPath)
jest.doMock(configPath, virtualConfig, { virtual: true })

config = jest.requireActual('./config')
Copy link
Owner Author

Choose a reason for hiding this comment

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

This part could probably be done more efficiently if we move the require-ing of the dynamic config file path to a separate function from the parser, but a lot of frustration went into this solution and I'm still a bit angry with Jest about it so it'll stay for now.

Copy link
Collaborator

@amoj amoj left a comment

Choose a reason for hiding this comment

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

You are missing one test i.e. you are not testing the case when fs.readFile throws an error and how that is handled. But that is not really important anyway =)

Really impressive work J!

@Sleavely Sleavely merged commit 19ac750 into master May 13, 2020
@Sleavely Sleavely deleted the config-tests branch May 19, 2020 10:45
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