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

[WIP] Failing tests if there are any unsilenced logs #761

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
4 changes: 2 additions & 2 deletions lib/WebpackConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,6 @@ class WebpackConfig {
}

createSharedEntry(name, file) {
logger.deprecation('Encore.createSharedEntry() is deprecated and will be removed in a future version, please use Encore.splitEntryChunks() or Encore.addCacheGroup() instead.');

if (this.shouldSplitEntryChunks) {
throw new Error('Using splitEntryChunks() and createSharedEntry() together is not supported. Use one of these strategies only to optimize your build.');
}
Expand All @@ -537,6 +535,8 @@ class WebpackConfig {
throw new Error('createSharedEntry() cannot be called multiple times: you can only create *one* shared entry.');
}

logger.deprecation('Encore.createSharedEntry() is deprecated and will be removed in a future version, please use Encore.splitEntryChunks() or Encore.addCacheGroup() instead.');

if (Array.isArray(file)) {
throw new Error('Argument 2 to createSharedEntry() must be a single string file: not an array of files. Try creating one file that requires/imports all the modules that should be included.');
}
Expand Down
17 changes: 17 additions & 0 deletions lib/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,23 @@ module.exports = {
log(`${chalk.bgYellow.black('DEPRECATION')} ${chalk.yellow(message)}`);
},

/**
* @returns {Array}
*/
getDeprecations() {
return messages.deprecation;
},

/**
* @returns {Array}
*/
getWarnings() {
return messages.warning;
},

/**
* @returns {Array}
*/
getMessages() {
return messages;
},
Expand Down
44 changes: 11 additions & 33 deletions test/WebpackConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const RuntimeConfig = require('../lib/config/RuntimeConfig');
const path = require('path');
const fs = require('fs');
const webpack = require('webpack');
const logger = require('../lib/logger');
const loggerAssert = require('./helpers/logger-assert');

function createConfig() {
const runtimeConfig = new RuntimeConfig();
Expand Down Expand Up @@ -139,11 +139,9 @@ describe('WebpackConfig object', () => {

it('You can omit the opening slash, but get a warning', () => {
const config = createConfig();
logger.reset();
logger.quiet();

config.setPublicPath('foo');
expect(logger.getMessages().warning).to.have.lengthOf(1);
loggerAssert.assertWarning('The value passed to setPublicPath() should *usually* start with "/" or be a full URL');
});
});

Expand Down Expand Up @@ -206,10 +204,8 @@ describe('WebpackConfig object', () => {
it('You can use an opening slash, but get a warning', () => {
const config = createConfig();

logger.reset();
logger.quiet();
config.setManifestKeyPrefix('/foo/');
expect(logger.getMessages().warning).to.have.lengthOf(1);
loggerAssert.assertWarning('The value passed to setManifestKeyPrefix "/foo/" starts with "/". This is allowed, but since the key prefix does not normally start with a "/"');
});
});

Expand Down Expand Up @@ -384,6 +380,7 @@ describe('WebpackConfig object', () => {
it('Calling twice throws an error', () => {
const config = createConfig();
config.createSharedEntry('vendor', 'jquery');
loggerAssert.assertDeprecation('Encore.createSharedEntry() is deprecated');

expect(() => {
config.createSharedEntry('vendor2', './main');
Expand Down Expand Up @@ -593,15 +590,6 @@ describe('WebpackConfig object', () => {
});

describe('configureBabel', () => {
beforeEach(() => {
logger.reset();
logger.quiet();
});

afterEach(() => {
logger.quiet(false);
});

it('Calling method sets it', () => {
const config = createConfig();
const testCallback = () => {};
Expand All @@ -619,7 +607,7 @@ describe('WebpackConfig object', () => {

it('Calling with "includeNodeModules" option', () => {
const config = createConfig();
config.configureBabel(() => {}, { include_node_modules: ['foo', 'bar'] });
config.configureBabel(() => {}, { includeNodeModules: ['foo', 'bar'] });

expect(config.babelOptions.exclude).to.be.a('Function');

Expand Down Expand Up @@ -668,26 +656,21 @@ describe('WebpackConfig object', () => {
config.runtimeConfig.babelRcFileExists = true;
config.configureBabel(() => {});

const warnings = logger.getMessages().warning;
expect(warnings).to.have.lengthOf(1);
expect(warnings[0]).to.contain('your app already provides an external Babel configuration');
loggerAssert.assertWarning('your app already provides an external Babel configuration');
});

it('Calling with a whitelisted option when .babelrc is present works fine', () => {
const config = createConfig();
config.runtimeConfig.babelRcFileExists = true;
config.configureBabel(null, { includeNodeModules: ['foo'] });
expect(logger.getMessages().warning).to.be.empty;
});

it('Calling with a non-whitelisted option when .babelrc is present displays a warning', () => {
const config = createConfig();
config.runtimeConfig.babelRcFileExists = true;
config.configureBabel(null, { useBuiltIns: 'foo' });

const warnings = logger.getMessages().warning;
expect(warnings).to.have.lengthOf(1);
expect(warnings[0]).to.contain('your app already provides an external Babel configuration');
loggerAssert.assertWarning('your app already provides an external Babel configuration');
});

it('Pass invalid config', () => {
Expand Down Expand Up @@ -716,15 +699,6 @@ describe('WebpackConfig object', () => {
});

describe('configureBabelPresetEnv', () => {
beforeEach(() => {
logger.reset();
logger.quiet();
});

afterEach(() => {
logger.quiet(false);
});

it('Calling method sets it', () => {
const config = createConfig();
const testCallback = () => {};
Expand Down Expand Up @@ -1348,6 +1322,7 @@ describe('WebpackConfig object', () => {

config.configureLoaderRule('eslint', callback);
expect(config.loaderConfigurationCallbacks['eslint']).to.equal(callback);
loggerAssert.assertWarning('Be careful when using Encore.configureLoaderRule');
});

it('Call method with a not supported loader', () => {
Expand All @@ -1356,6 +1331,7 @@ describe('WebpackConfig object', () => {
expect(() => {
config.configureLoaderRule('reason');
}).to.throw('Loader "reason" is not configurable. Valid loaders are "javascript", "css", "images", "fonts", "sass", "less", "stylus", "vue", "eslint", "typescript", "handlebars" and the aliases "js", "ts", "scss".');
loggerAssert.assertWarning('Be careful when using Encore.configureLoaderRule');
});

it('Call method with not a valid callback', () => {
Expand All @@ -1364,10 +1340,12 @@ describe('WebpackConfig object', () => {
expect(() => {
config.configureLoaderRule('eslint');
}).to.throw('Argument 2 to configureLoaderRule() must be a callback function.');
loggerAssert.assertWarning('Be careful when using Encore.configureLoaderRule');

expect(() => {
config.configureLoaderRule('eslint', {});
}).to.throw('Argument 2 to configureLoaderRule() must be a callback function.');
loggerAssert.assertWarning('Be careful when using Encore.configureLoaderRule');
});
});

Expand Down
28 changes: 28 additions & 0 deletions test/_unsilencedLogsCheck.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* This file is part of the Symfony Webpack Encore package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

'use strict';

const logger = require('../lib/logger');

beforeEach(function() {
logger.quiet();
});

afterEach(function() {
if (logger.getDeprecations().length > 0) {
this.test.error(new Error(`There were ${logger.getDeprecations().length} unexpected deprecation log messages: \n${logger.getDeprecations().join('\n')}`));
}

if (logger.getWarnings().length > 0) {
this.test.error(new Error(`There were ${logger.getWarnings().length} unexpected warning log messages: \n${logger.getWarnings().join('\n')}`));
}

logger.reset();
});
41 changes: 18 additions & 23 deletions test/config-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ const ManifestPlugin = require('webpack-manifest-plugin');
const { CleanWebpackPlugin } = require('clean-webpack-plugin');
const webpack = require('webpack');
const path = require('path');
const logger = require('../lib/logger');
const loggerAssert = require('./helpers/logger-assert');

const isWindows = (process.platform === 'win32');

function createConfig(runtimeConfig = null) {
function createConfig(runtimeConfig = null, disableSingleRuntimeChunk = true) {
runtimeConfig = runtimeConfig ? runtimeConfig : new RuntimeConfig();

if (null === runtimeConfig.context) {
Expand All @@ -37,7 +37,12 @@ function createConfig(runtimeConfig = null) {
runtimeConfig.babelRcFileExists = false;
}

return new WebpackConfig(runtimeConfig);
const config = new WebpackConfig(runtimeConfig);
if (disableSingleRuntimeChunk) {
config.disableSingleRuntimeChunk();
}

return config;
}

function findPlugin(pluginConstructor, plugins) {
Expand Down Expand Up @@ -167,6 +172,7 @@ describe('The config-generator function', () => {
// pretend we're installed to a subdirectory
config.setPublicPath('/subdirectory/build');
config.setManifestKeyPrefix('/build');
loggerAssert.assertWarning('The value passed to setManifestKeyPrefix "/build" starts with "/"');

const actualConfig = configGenerator(config);

Expand Down Expand Up @@ -372,10 +378,6 @@ describe('The config-generator function', () => {
});

it('enableEslintLoader("extends-name")', () => {
before(() => {
logger.reset();
});

const config = createConfig();
config.addEntry('main', './main');
config.publicPath = '/';
Expand All @@ -384,7 +386,7 @@ describe('The config-generator function', () => {

const actualConfig = configGenerator(config);

expect(JSON.stringify(logger.getMessages().deprecation)).to.contain('enableEslintLoader: Extending from a configuration is deprecated, please use a configuration file instead. See https://eslint.org/docs/user-guide/configuring for more information.');
loggerAssert.assertDeprecation('enableEslintLoader: Extending from a configuration is deprecated, please use a configuration file instead. See https://eslint.org/docs/user-guide/configuring for more information.');
expect(JSON.stringify(actualConfig.module.rules)).to.contain('eslint-loader');
expect(JSON.stringify(actualConfig.module.rules)).to.contain('extends-name');
});
Expand Down Expand Up @@ -1068,15 +1070,6 @@ describe('The config-generator function', () => {
});

describe('Test shouldUseSingleRuntimeChunk', () => {
before(() => {
logger.reset();
logger.quiet();
});

after(() => {
logger.quiet(false);
});

it('Set to true', () => {
const config = createConfig();
config.outputPath = '/tmp/public/build';
Expand All @@ -1085,7 +1078,6 @@ describe('The config-generator function', () => {

const actualConfig = configGenerator(config);
expect(actualConfig.optimization.runtimeChunk).to.equal('single');
expect(logger.getMessages().deprecation).to.be.empty;
});

it('Set to false', () => {
Expand All @@ -1096,28 +1088,27 @@ describe('The config-generator function', () => {

const actualConfig = configGenerator(config);
expect(actualConfig.optimization.runtimeChunk).to.be.undefined;
expect(logger.getMessages().deprecation).to.be.empty;
});

it('Not set + createSharedEntry()', () => {
const config = createConfig();
const config = createConfig(null, false);
config.outputPath = '/tmp/public/build';
config.setPublicPath('/build/');
config.createSharedEntry('foo', 'bar.js');

const actualConfig = configGenerator(config);
expect(actualConfig.optimization.runtimeChunk.name).to.equal('manifest');
expect(JSON.stringify(logger.getMessages().deprecation)).to.contain('the recommended setting is Encore.enableSingleRuntimeChunk()');
loggerAssert.assertDeprecation('the recommended setting is Encore.enableSingleRuntimeChunk()');
});

it('Not set without createSharedEntry()', () => {
const config = createConfig();
const config = createConfig(null, false);
config.outputPath = '/tmp/public/build';
config.setPublicPath('/build/');

const actualConfig = configGenerator(config);
expect(actualConfig.optimization.runtimeChunk).to.be.undefined;
expect(JSON.stringify(logger.getMessages().deprecation)).to.contain('the recommended setting is Encore.enableSingleRuntimeChunk()');
loggerAssert.assertDeprecation('the recommended setting is Encore.enableSingleRuntimeChunk()');
});
});

Expand Down Expand Up @@ -1148,6 +1139,10 @@ describe('The config-generator function', () => {
config.enableSingleRuntimeChunk();
});

afterEach(function() {
loggerAssert.assertWarning('Be careful when using Encore.configureLoaderRule');
});

it('configure rule for "javascript"', () => {
config.configureLoaderRule('javascript', (loaderRule) => {
loaderRule.test = /\.m?js$/;
Expand Down
3 changes: 3 additions & 0 deletions test/config/path-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const WebpackConfig = require('../../lib/WebpackConfig');
const RuntimeConfig = require('../../lib/config/RuntimeConfig');
const pathUtil = require('../../lib/config/path-util');
const process = require('process');
const loggerAssert = require('../helpers/logger-assert');

function createConfig() {
const runtimeConfig = new RuntimeConfig();
Expand Down Expand Up @@ -54,6 +55,8 @@ describe('path-util getContentBase()', () => {

const actualContentBase = pathUtil.getContentBase(config);
expect(actualContentBase).to.equal(isWindows ? 'C:\\tmp\\public' : '/tmp/public');

loggerAssert.assertWarning('The value passed to setManifestKeyPrefix "/build/" starts with "/". This is allowed, but');
});

it('contentBase is calculated correctly with no public path', function() {
Expand Down