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.

Sign up for GitHub

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

getConfig will assign default options to generated config #429

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

getConfig will assign default options to generated config #429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a190c55
Assign default properties to generated config
Oct 26, 2017
ee9ad87
Merge branch 'master' into fix-409
Oct 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/utils/setConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var path = require( 'path' )
var userHome = require( 'user-home' )
var pathIsAbsolute = require( 'path-is-absolute' )
var stripJsonComments = require( 'strip-json-comments' )
var defaults = require( 'lodash.defaults' )
var Glob = require( 'glob' ).Glob

// @TODO i just this sloppy just to fix some stuff
Expand Down Expand Up @@ -146,10 +147,11 @@ var setConfig = function( configpath ) {
} ).minimatch
} )

// make sure indentPref is set no matter what
returnConfig.indentPref = returnConfig.indentPref || false
// assign default properties to generated config
// to make sure all properties are present
returnConfig = defaults( returnConfig, this.config || {} )

// 5, just return the default config if nothing found
// 5, return complete config
return returnConfig
}

Expand Down
10 changes: 6 additions & 4 deletions test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@
* lets pull in what we're testing here
*/

var fs = require( 'fs' )
var assert = require( 'assert' )
require( 'chai' ).should() // add should assertions on top
var chokidar = require( 'chokidar' )
var touch = require( 'touch' )
var sinon = require( 'sinon' )
var app = require( '../index' )().create()
var stripJsonComments = require( 'strip-json-comments' )

// turn on strict mode from this point and turn off unnecessary logging
app.state.quiet = true
Expand Down Expand Up @@ -387,10 +385,14 @@ describe( 'Utility Methods: ', function() {
process.argv[2] = '-c'
process.argv[3] = '.stylintrc'
var testMethod = app.setConfig( '.stylintrc' )
var testConfig = JSON.parse( stripJsonComments( fs.readFileSync( process.cwd() + '/.stylintrc', 'utf-8' ) ) )
var testConfig = app.setConfig( process.cwd() + '/.stylintrc' )

it( 'return complete config when not complete config is passed in', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice test! Can we paraphrase a bit to avoid ambiguity with what complete means here?

Perhaps something like:

it( 'return config with all properties when passed config with missing properties', function() {

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I'll update the PR. Thnx.

assert.ok( testConfig.reporter )
} )

it( 'update config state if passed a valid path', function() {
assert.deepEqual( testMethod, testConfig )
assert.equal( Object.keys( testConfig ).length, Object.keys( testMethod ).length )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we changing this test? If we must change this, we should still check for deep equality between two objects since doing key comparisons can lead to false positives.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Trying to remember this one.. 😣

I think I changed it since the important part is that they should contain the same amount of options now, not the exact same values since they may vary. I'll try to dig out my old notes.

} )

it( 'throw if passed invalid path', function() {
Expand Down