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

Jump to bottom

[#4] Set up the standard linter on CI #5

Merged
merged 1 commit into from
Sep 2, 2017
Merged

[#4] Set up the standard linter on CI #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = function (content) {
var map = this._compilation[MAP_KEY] = this._compilation[MAP_KEY] || {}

// Override emitFile function to get an url from file-loader
this.emitFile = function(url, fileContent) {
this.emitFile = function (url, fileContent) {
map[this.resource] = path.join(this.options.output.publicPath || '/', url)
originEmitFile.call(this, url, fileContent)
}.bind(this)
Expand Down
12 changes: 10 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
"description": "file-loader that stores a map of original file names to file names with hashes in the compilation stats",
"main": "index.js",
"scripts": {
"test": "env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js --watch",
"test-ci": "env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js"
"lint": "standard",
"lint-fix": "standard --fix",
"test": "npm run lint && env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js --watch",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will only run the linter one time, and then watch the unit tests. If we'd prefer, I can set up chokidar to run both tasks when files change.

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, maybe it's better to create separate lint-watch task? There is an amazing https://www.npmjs.com/package/eslint-watch that does this job well.

As a side note. I think that the previous task naming is unfortunate. If I (or let's say CI) run npm test I expect to run it in a single mode. Maybe we should move watch mode to npm run test-watch for consistency sake?

"test-ci": "npm run lint && env NODE_ENV=test mocha --require ./test/power_assert_loader.js test/basic.js test/integration.js"
},
"repository": {
"type": "git",
Expand All @@ -29,6 +31,12 @@
"rewire-test-helpers": "^0.2.0",
"rimraf": "^2.4.3",
"sinon": "^1.17.2",
"standard": "^10.0.2",
"webpack": "^1.12.2"
},
"standard": {
"env": [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note that this applies the mocha env to all files being linted, including source (non-test) files. If we'd prefer, we can switch to using comments at the head of files

Copy link
Owner

Choose a reason for hiding this comment

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

I think it's ok, I doubt that describe, it, etc. would ever leak to the source code, so it's not a big deal.

"mocha"
]
}
}
33 changes: 16 additions & 17 deletions test/basic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ var rewireHelpers = require('rewire-test-helpers')
var staticFileLoader = rewire('../')
var staticFileLoaderKey = staticFileLoader.key

describe('basic tests', function() {
var getContext = function(overrides) {
describe('basic tests', function () {
var getContext = function (overrides) {
overrides = overrides || {}
var compilation = {}

Expand All @@ -15,7 +15,7 @@ describe('basic tests', function() {
}

return {
emitFile: function() {},
emitFile: function () {},
resource: '/a/b/c.png',
_compilation: compilation,
options: {
Expand All @@ -27,56 +27,55 @@ describe('basic tests', function() {
}

rewireHelpers.injectDependenciesFilter(staticFileLoader, {
fileLoader: function() {
fileLoader: function () {
this.emitFile('abc.png', 'abc')
}
})

it('do not throw an error when the map is not exists in the context', function() {
assert.doesNotThrow(function() {
it('do not throw an error when the map is not exists in the context', function () {
assert.doesNotThrow(function () {
staticFileLoader.call(getContext())
})
})

it('adds a file to the map when the map is exists in the context', function() {
it('adds a file to the map when the map is exists in the context', function () {
var map = {a: 'a'}
var context = getContext({map: map})
staticFileLoader.call(context)
assert(Object.keys(map).length == 2)
assert(Object.keys(map).length === 2)
})

it('calls file-loader with passed arguments', function() {
it('calls file-loader with passed arguments', function () {
var fileLoader = sinon.spy()
rewireHelpers.rewired(staticFileLoader, {fileLoader: fileLoader}, function() {
rewireHelpers.rewired(staticFileLoader, {fileLoader: fileLoader}, function () {
staticFileLoader.call(getContext(), 'qwe')
assert(fileLoader.calledWith('qwe'))
})
})

it('calls file-loader with passed arguments and stores a file to the map', function() {
it('calls file-loader with passed arguments and stores a file to the map', function () {
var map = {}
var context = getContext({map: map})
staticFileLoader.call(context)
assert(map['/a/b/c.png'] === '/bundles/abc.png')
})


it('restores the original emitFile function', function() {
it('restores the original emitFile function', function () {
var context = getContext()
var emitFile = context.emitFile
staticFileLoader.call(context)
assert(emitFile === context.emitFile)
})

it('returns file-loader result', function() {
var fileLoader = function() { return 'asd' }
rewireHelpers.rewired(staticFileLoader, {fileLoader: fileLoader}, function() {
it('returns file-loader result', function () {
var fileLoader = function () { return 'asd' }
rewireHelpers.rewired(staticFileLoader, {fileLoader: fileLoader}, function () {
var result = staticFileLoader.call(getContext())
assert(result === 'asd')
})
})

it('sets raw = true to the export', function() {
it('sets raw = true to the export', function () {
assert(staticFileLoader.raw)
})
})
28 changes: 17 additions & 11 deletions test/integration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@ var staticFileLoaderKey = require('../').key
var webpack = require('webpack')
var rmrf = require('rimraf')

describe('integration tests', function() {
beforeEach(function() {
describe('integration tests', function () {
beforeEach(function () {
fs.symlinkSync(process.cwd(), path.join(process.cwd(), 'node_modules', 'static-file-loader'))
})

afterEach(function(done) {
afterEach(function (done) {
fs.unlinkSync(path.join(process.cwd(), 'node_modules', 'static-file-loader'))
rmrf(path.join(__dirname, 'dist'), done)
})

context('when publicPath is specified', function() {
it('stores files map in the compilation stats', function(done) {
context('when publicPath is specified', function () {
it('stores files map in the compilation stats', function (done) {
var compiler = webpack({
context: __dirname,
entry: './fixtures/index.js',
Expand All @@ -26,24 +26,27 @@ describe('integration tests', function() {
publicPath: '/bundles'
}
})
compiler.run(function(err, stats) {
compiler.run(function (err, stats) {
if (err) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Standard style requires all errors be handled. I rethrow here, which will fail the tests, but I could instead call assert.fail if we prefer.

Copy link
Owner

Choose a reason for hiding this comment

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

I think that's enough 👍

throw err
}
var staticFiles = stats.compilation[staticFileLoaderKey]
var fileNames = Object.keys(staticFiles)
assert.deepEqual(fileNames.sort(), [
path.join(__dirname, 'fixtures', 'static', 'a.gif'),
path.join(__dirname, 'fixtures', 'static', 'b.gif'),
path.join(__dirname, 'fixtures', 'static', 'c.gif')
])
fileNames.forEach(function(fileName) {
fileNames.forEach(function (fileName) {
assert(staticFiles[fileName].match(/\/bundles\/\w+.gif$/))
})
done()
})
})
})

context('when publicPath is not specified', function() {
it('stores files map in the compilation stats', function(done) {
context('when publicPath is not specified', function () {
it('stores files map in the compilation stats', function (done) {
var compiler = webpack({
context: __dirname,
entry: './fixtures/index.js',
Expand All @@ -52,15 +55,18 @@ describe('integration tests', function() {
filename: 'bundle.js'
}
})
compiler.run(function(err, stats) {
compiler.run(function (err, stats) {
if (err) {
throw err
}
var staticFiles = stats.compilation[staticFileLoaderKey]
var fileNames = Object.keys(staticFiles)
assert.deepEqual(fileNames.sort(), [
path.join(__dirname, 'fixtures', 'static', 'a.gif'),
path.join(__dirname, 'fixtures', 'static', 'b.gif'),
path.join(__dirname, 'fixtures', 'static', 'c.gif')
])
fileNames.forEach(function(fileName) {
fileNames.forEach(function (fileName) {
assert(staticFiles[fileName].match(/\/\w+.gif$/))
})
done()
Expand Down