Added unit tests for basic configurations

[ekulabuhov]

What kind of change does this PR introduce?
Build related change

Did you add tests for your changes?
Yes

If relevant, did you update the README?
Not relevant

Summary
This PR introduces unit testing setup we can use to check for regressions when adding new features to style-loader. Hopefully, this can provide a base for PR authors to test against.

The setup consists of:

• memory-fs to avoid writing to file system and to collocate input files
• jsdom to provide lightweight DOM implementation

Each test is running fully setup webpack compiler. This allows for great level of flexibility. Included are 5 tests for basic configurations shown in the Readme file.

Does this PR introduce a breaking change?
No

Other information

[jsf-clabot]

All committers have signed the CLA.

[joshwiens]

@ekulabuhov - This is no longer a required check and is not blocking this pull request. As i said in slack, all of it's complaints are invalidated the second we apply webpack-defaults anyway nor is codacy.com our hosted static analysis tooling of choice.

[joshwiens]

@ekulabuhov - one thing of note.

If we are going to add tests and i'm all for it, best to do it in a way that is beneficial for all future pull requests.

• Mocha || Jest + Chai ( if an assertion lib is necessary )
• Grab the webpac-contrib standard travis file
• Webpack dependency from devDep => peerDep (Travis installs this)

[bebraw]

I would split the test utilities from test/basicText.js to a file of their own. That will keep the tests easier to read.

Nice work overall. 👍

[bebraw]

Good changes. 👍

[ekulabuhov]

@d3viant0ne I've tried moving webpack to peerDep but it didn't install it on Travis: https://travis-ci.org/ekulabuhov/style-loader/jobs/206042662

Other than that, I think I'm ok with merging this.

[joshwiens]

Requires, the full travis file. I've commented out the bit's that will come with defaults but are outside the scope of this PR.

.travis.yml

sudo: false
language: node_js
branches:
  only:
    - master
matrix:
  fast_finish: true
  include:
    # - os: linux
    #   node_js: '7'
    #   env: WEBPACK_VERSION="2.2.0" BITHOUND_CHECK=true JOB_PART=lint
    - os: linux
      node_js: '7'
      env: WEBPACK_VERSION="2.2.0" JOB_PART=test
    - os: linux
      node_js: '4.3'
      env: WEBPACK_VERSION="2.2.0" JOB_PART=test
    - os: linux
      node_js: '6'
      env: WEBPACK_VERSION="2.2.0" JOB_PART=test
    # - os: linux
    #   node_js: '7'
    #   env: WEBPACK_VERSION="2.2.0" JOB_PART=coverage
before_install:
  - nvm --version
  - node --version
before_script:
  - if [ "$WEBPACK_VERSION" ]; then yarn add webpack@^$WEBPACK_VERSION; fi
  # - if [ "$BITHOUND_CHECK" ]; then npm install -g bithound; bithound check [email protected]:$TRAVIS_REPO_SLUG.git; fi
script:
  - yarn run travis:$JOB_PART
after_success:
  - bash <(curl -s https://codecov.io/bash)

package.json

{
  "name": "style-loader",
  "version": "0.13.2",
  "author": "Tobias Koppers @sokra",
  "description": "style loader module for webpack",
  "devDependencies": {
    "css-loader": "~0.8.0",
    "file-loader": "^0.10.1",
    "jsdom": "^9.11.0",
    "memory-fs": "^0.4.1",
    "mocha": "^3.2.0",
    "webpack": "^2.2.1"
  },
  "repository": {
    "type": "git",
    "url": "[email protected]:webpack/style-loader.git"
  },
  "license": "MIT",
  "dependencies": {
    "loader-utils": "^1.0.2"
  },
  "scripts": {
    "test": "mocha",
    "travis:test": "yarn run test",
  }
}

[joshwiens]

@ekulabuhov - I've enabled style-loader in travis-ci.org. Should run next time anything changes.

[joshwiens]

.gitattributes

# Treats the lock file as binary & prevents conflict hell
yarn.lock -diff

[ekulabuhov]

@d3viant0ne - I've tried moving webpack to peerDependencies again. I can see it working in Travis (where it explicitly install it), but it fails when running mocha locally. Both NPM and yarn seem to ignore peerDependencies. Wouldn't it require each developer to install it manually?

[bebraw]

I know from experience that npm 2 will be problematic with peerDependencies. It would not be a big problem for me if we pushed webpack to devDependencies too to make to get this merged. 👍

[joshwiens]

At the moment we are only testing against a single version of webpack. We have the same issue in less-loader, devDep for the time being is fine.

[bebraw]

Ah, yeah. I see your point now. Basically you want to test against multiple versions of webpack later on. That's a good separate problem to solve.

[ekulabuhov]

Ok, thanks! Fixed Node v4 issues. Changed travis.yml from v4.3 to v4 to capture latest version of node. Got tests passing on all 3 versions. Good to go 👍

Should I squash it before merging?

[joshwiens]

That was actually 4.3 for a reason (Minimum supported nodejs version for Webpack).

[ekulabuhov]

Ah, ok, didn't think about that. Reverted. Looked at css-loader and it's set to v4.7.

[joshwiens]

I'll update it. At one point the minimum version was 4.7 ( long story )

[joshwiens]

@bebraw - Unless @ekulabuhov has something else he wants to add to this, i'd say ship it.

Trying to kill that Codacy PR check with fire, for the time being just ignore it.

[ekulabuhov]

I'm happy to merge this. But it will definately break: #122

[joshwiens]

Pull request validations > any code fix imo. Ship it :)

[bebraw]

👍 Nice work.

[michael-ciniawsky]

👍