Skip to content

refactor: add config.reset() and .resetForTesting()#641

Merged
freitagbr merged 1 commit intomasterfrom
refactor-reset-config-options
Jan 8, 2017
Merged

refactor: add config.reset() and .resetForTesting()#641
freitagbr merged 1 commit intomasterfrom
refactor-reset-config-options

Conversation

@nfischer
Copy link
Copy Markdown
Member

@nfischer nfischer commented Jan 5, 2017

This adds .reset() and .resetForTesting() to shell.config and use .resetForTesting() as a standard set-up for unit tests.

This also adds npm run test-no-coverage, since testing with coverage every time gets pretty slow.

.reset() is implicitly tested by checking default values for shell.config. .resetForTesting() is relied upon by all unit tests.

@nfischer nfischer added this to the v0.7.x milestone Jan 5, 2017
@nfischer nfischer requested a review from freitagbr January 5, 2017 08:39
@nfischer
Copy link
Copy Markdown
Member Author

nfischer commented Jan 8, 2017

@freitagbr this should be passing all tests now.

Let me know if you think we should document shell.config.reset() as part of the public API.

src/common.js Outdated
Object.assign(this, DEFAULT_CONFIG);
},
resetForTesting: function () {
Object.assign(this, DEFAULT_CONFIG);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These should be objectAssign.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Whoops, forgot to push the fix. Yeah, this should work now

@nfischer nfischer force-pushed the refactor-reset-config-options branch from 965b041 to 91e30bc Compare January 8, 2017 02:16
@freitagbr
Copy link
Copy Markdown
Contributor

freitagbr commented Jan 8, 2017

Let's also add documentation for shell.config.reset. Something simple like:

shell.config.reset()

Reset the configuration to the following defaults:

{
    foo: 'bar',
    ...
}

Other than that, LGTM.

@nfischer nfischer force-pushed the refactor-reset-config-options branch from 91e30bc to 040f743 Compare January 8, 2017 04:05
@nfischer nfischer force-pushed the refactor-reset-config-options branch from 040f743 to 8201591 Compare January 8, 2017 04:43
Add .reset() and .resetForTesting() to shell.config and use .resetForTesting()
as a standard set-up for unit tests.
@nfischer nfischer force-pushed the refactor-reset-config-options branch from 8201591 to 85c637a Compare January 8, 2017 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants