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

Fix windows bugs & tests #1965

Merged
merged 24 commits into from Sep 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 0 additions & 6 deletions appveyor.yml
Expand Up @@ -3,7 +3,6 @@ environment:
matrix:
- nodejs_version: "6"
Copy link
Member Author

Choose a reason for hiding this comment

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

Travis already tests Node 6 compatibility and the tests that break on windows that don't break on Travis are usually related to the filesystem, which is the same on Node 8.

Removing this would make it as fast as Travis, this way we don't have to wait hours before AppVeyor catches up

- nodejs_version: "8"
- nodejs_version: "10"
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't we test on nodejs 10 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it takes too long to run all platforms and node 10 should generally work if node 8 works.

Travis is fine, cuz it runs the builds in parallel, AppVeyor runs one platform after another taking ages to finish.

Besides node specific issues should be caught by Travis anyways, Appveyor is just to catch windows errors which should be the same on pretty much every node version (unless there is a bug inside node or some native package we use (which we try to avoid))

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, that makes sense. Hopefully, there won't rise any node-specific windows errors like that edge case.


# Install scripts. (runs after repo cloning)
install:
Expand All @@ -20,11 +19,6 @@ install:
- rustc -Vv
- cargo -V

# Restore symlinks from git
# https://github.com/appveyor/ci/issues/650
- cmd: git config core.symlinks true
- cmd: git reset --hard

# Post-install test scripts.
test_script:
# Output useful info for debugging.
Expand Down
2 changes: 1 addition & 1 deletion src/Bundler.js
Expand Up @@ -19,7 +19,7 @@ const installPackage = require('./utils/installPackage');
const bundleReport = require('./utils/bundleReport');
const prettifyTime = require('./utils/prettifyTime');
const getRootDir = require('./utils/getRootDir');
const glob = require('fast-glob');
const {glob} = require('./utils/glob');

/**
* The Bundler is the main entry point. It resolves and loads assets,
Expand Down
3 changes: 1 addition & 2 deletions src/FSCache.js
Expand Up @@ -4,8 +4,7 @@ const md5 = require('./utils/md5');
const objectHash = require('./utils/objectHash');
const pkg = require('../package.json');
const logger = require('./Logger');
const glob = require('fast-glob');
const isGlob = require('is-glob');
const {isGlob, glob} = require('./utils/glob');

// These keys can affect the output, so if they differ, the cache should not match
const OPTION_KEYS = ['publicURL', 'minify', 'hmr', 'target', 'scopeHoist'];
Expand Down
2 changes: 1 addition & 1 deletion src/Parser.js
@@ -1,7 +1,7 @@
const path = require('path');
const RawAsset = require('./assets/RawAsset');
const GlobAsset = require('./assets/GlobAsset');
const isGlob = require('is-glob');
const {isGlob} = require('./utils/glob');

class Parser {
constructor(options = {}) {
Expand Down
4 changes: 2 additions & 2 deletions src/Resolver.js
@@ -1,7 +1,7 @@
const builtins = require('./builtins');
const nodeBuiltins = require('node-libs-browser');
const path = require('path');
const isGlob = require('is-glob');
const {isGlob} = require('./utils/glob');
const fs = require('./utils/fs');
const micromatch = require('micromatch');

Expand Down Expand Up @@ -147,7 +147,7 @@ class Resolver {

default:
// Module
return path.normalize(filename);
return filename;
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/assets/GlobAsset.js
@@ -1,7 +1,7 @@
const Asset = require('../Asset');
const glob = require('fast-glob');
const micromatch = require('micromatch');
const path = require('path');
const {glob} = require('../utils/glob');

class GlobAsset extends Asset {
constructor(name, options) {
Expand Down
3 changes: 1 addition & 2 deletions src/assets/StylusAsset.js
Expand Up @@ -4,8 +4,7 @@ const localRequire = require('../utils/localRequire');
const Resolver = require('../Resolver');
const fs = require('../utils/fs');
const {dirname, resolve, relative} = require('path');
const isGlob = require('is-glob');
const glob = require('fast-glob');
const {isGlob, glob} = require('../utils/glob');

const URL_RE = /^(?:url\s*\(\s*)?['"]?(?:[#/]|(?:https?:)?\/\/)/i;

Expand Down
18 changes: 18 additions & 0 deletions src/utils/glob.js
@@ -0,0 +1,18 @@
const isGlob = require('is-glob');
const fastGlob = require('fast-glob');

function normalisePath(p) {
return p.replace(/\\/g, '/');
}

exports.isGlob = function(p) {
return isGlob(normalisePath(p));
};

exports.glob = function(p, options) {
return fastGlob(normalisePath(p), options);
};

exports.glob.sync = function(p, options) {
return fastGlob.sync(normalisePath(p), options);
};
2 changes: 1 addition & 1 deletion test/asset.js
Expand Up @@ -24,7 +24,7 @@ describe('Asset', () => {

it('should support overriding the filename of the root bundle', async function() {
const outFile = 'custom-out-file.html';
await bundle(__dirname + '/integration/html/index.html', {
await bundle(path.join(__dirname, '/integration/html/index.html'), {
outFile
});

Expand Down
5 changes: 3 additions & 2 deletions test/autoinstall.js
Expand Up @@ -2,13 +2,14 @@ const assert = require('assert');
const install = require('../src/utils/installPackage');
const fs = require('../src/utils/fs');
const {ncp, rimraf} = require('./utils');
const inputDirPath = __dirname + '/input';
const path = require('path');
const inputDirPath = path.join(__dirname, '/input');

describe('autoinstall', function() {
beforeEach(async function() {
// Setup (clear the input dir and move integration test in)
await rimraf(inputDirPath, {});
await ncp(__dirname + '/integration/babel-default', inputDirPath);
await ncp(path.join(__dirname, '/integration/babel-default'), inputDirPath);
});

it('should install lodash using npm and save dev dependency to package.json', async function() {
Expand Down
19 changes: 12 additions & 7 deletions test/bundler.js
@@ -1,18 +1,21 @@
const assert = require('assert');
const sinon = require('sinon');
const path = require('path');
const {assertBundleTree, bundle, bundler, nextBundle} = require('./utils');

describe('bundler', function() {
it('should bundle once before exporting middleware', async function() {
let b = bundler(__dirname + '/integration/bundler-middleware/index.js');
let b = bundler(
path.join(__dirname, '/integration/bundler-middleware/index.js')
);
b.middleware();

await nextBundle(b);
assert(b.entryAssets);
});

it('should defer bundling if a bundle is pending', async () => {
const b = bundler(__dirname + '/integration/html/index.html');
const b = bundler(path.join(__dirname, '/integration/html/index.html'));
b.pending = true; // bundle in progress
const spy = sinon.spy(b, 'bundle');

Expand All @@ -30,15 +33,15 @@ describe('bundler', function() {
});

it('should enforce asset type path to be a string', () => {
const b = bundler(__dirname + '/integration/html/index.html');
const b = bundler(path.join(__dirname, '/integration/html/index.html'));

assert.throws(() => {
b.addAssetType('.ext', {});
}, 'should be a module path');
});

it('should enforce setup before bundling', () => {
const b = bundler(__dirname + '/integration/html/index.html');
const b = bundler(path.join(__dirname, '/integration/html/index.html'));
b.farm = true; // truthy

assert.throws(() => {
Expand All @@ -52,8 +55,8 @@ describe('bundler', function() {

it('should support multiple entry points', async function() {
let b = await bundle([
__dirname + '/integration/multi-entry/one.html',
__dirname + '/integration/multi-entry/two.html'
path.join(__dirname, '/integration/multi-entry/one.html'),
path.join(__dirname, '/integration/multi-entry/two.html')
]);

await assertBundleTree(b, [
Expand All @@ -76,7 +79,9 @@ describe('bundler', function() {
});

it('should support multiple entry points as a glob', async function() {
let b = await bundle(__dirname + '/integration/multi-entry/*.html');
let b = await bundle(
path.join(__dirname, '/integration/multi-entry/*.html')
);

await assertBundleTree(b, [
{
Expand Down
44 changes: 27 additions & 17 deletions test/contentHashing.js
@@ -1,63 +1,73 @@
const assert = require('assert');
const path = require('path');
const fs = require('../src/utils/fs');
const {bundle, rimraf, ncp} = require('./utils');

describe('content hashing', function() {
beforeEach(async function() {
await rimraf(__dirname + '/input');
await rimraf(path.join(__dirname, '/input'));
});

it('should update content hash when content changes', async function() {
await ncp(__dirname + '/integration/html-css', __dirname + '/input');
await ncp(
path.join(__dirname, '/integration/html-css'),
path.join(__dirname, '/input')
);

await bundle(__dirname + '/input/index.html', {
await bundle(path.join(__dirname, '/input/index.html'), {
production: true
});

let html = await fs.readFile(__dirname + '/dist/index.html', 'utf8');
let html = await fs.readFile(
path.join(__dirname, '/dist/index.html'),
'utf8'
);
let filename = html.match(
/<link rel="stylesheet" href="[/\\]{1}(input\.[a-f0-9]+\.css)">/
)[1];
assert(await fs.exists(__dirname + '/dist/' + filename));
assert(await fs.exists(path.join(__dirname, '/dist/', filename)));

await fs.writeFile(
__dirname + '/input/index.css',
path.join(__dirname, '/input/index.css'),
'body { background: green }'
);

await bundle(__dirname + '/input/index.html', {
await bundle(path.join(__dirname, '/input/index.html'), {
production: true
});

html = await fs.readFile(__dirname + '/dist/index.html', 'utf8');
html = await fs.readFile(path.join(__dirname, '/dist/index.html'), 'utf8');
let newFilename = html.match(
/<link rel="stylesheet" href="[/\\]{1}(input\.[a-f0-9]+\.css)">/
)[1];
assert(await fs.exists(__dirname + '/dist/' + newFilename));
assert(await fs.exists(path.join(__dirname, '/dist/', newFilename)));

assert.notEqual(filename, newFilename);
});

it('should update content hash when raw asset changes', async function() {
await ncp(__dirname + '/integration/import-raw', __dirname + '/input');
await ncp(
path.join(__dirname, '/integration/import-raw'),
path.join(__dirname, '/input')
);

await bundle(__dirname + '/input/index.js', {
await bundle(path.join(__dirname, '/input/index.js'), {
production: true
});

let js = await fs.readFile(__dirname + '/dist/index.js', 'utf8');
let js = await fs.readFile(path.join(__dirname, '/dist/index.js'), 'utf8');
let filename = js.match(/\/(test\.[0-9a-f]+\.txt)/)[1];
assert(await fs.exists(__dirname + '/dist/' + filename));
assert(await fs.exists(path.join(__dirname, '/dist/', filename)));

await fs.writeFile(__dirname + '/input/test.txt', 'hello world');
await fs.writeFile(path.join(__dirname, '/input/test.txt'), 'hello world');

await bundle(__dirname + '/input/index.js', {
await bundle(path.join(__dirname, '/input/index.js'), {
production: true
});

js = await fs.readFile(__dirname + '/dist/index.js', 'utf8');
js = await fs.readFile(path.join(__dirname, '/dist/index.js'), 'utf8');
let newFilename = js.match(/\/(test\.[0-9a-f]+\.txt)/)[1];
assert(await fs.exists(__dirname + '/dist/' + newFilename));
assert(await fs.exists(path.join(__dirname, '/dist/', newFilename)));

assert.notEqual(filename, newFilename);
});
Expand Down