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

feat: Adapt to serverless@3 logging interface #174

Merged
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ Breaking changes are introduced when going from version 3.x.x to 4.x.x:
* This is because your environment variables might not be stored in dotenv files in all environments.
* Setting `required.file` to `true` will continue to cause the plugin to halt if no dotenv files are found.

## 3.12.x

* feat: Adapt to `serverless@3` logging interface ([#174](https://github.com/neverendingqs/serverless-dotenv-plugin/pull/174))

## 3.11.x

* feat: add support for `serverless@3`. ([#178](https://github.com/neverendingqs/serverless-dotenv-plugin/pull/178))
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "serverless-dotenv-plugin",
"version": "3.11.1",
"version": "3.12.0",
"description": "Preload environment variables with dotenv into serverless.",
"main": "src/index.js",
"files": [
Expand Down
15 changes: 12 additions & 3 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const errorTypes = {
};

class ServerlessPlugin {
constructor(serverless, options) {
constructor(serverless, options, v3Utils) {
this.serverless = serverless;
this.serverless.service.provider.environment =
this.serverless.service.provider.environment || {};
Expand All @@ -37,12 +37,21 @@ class ServerlessPlugin {
);
}

this.v3Utils = v3Utils;
this.loadEnv(this.getEnvironment(options));
}

log(...args) {
log(msg) {
if (this.config.logging) {
this.serverless.cli.log(...args);
if (this.v3Utils) {
if (msg.includes('WARNING')) {
this.v3Utils.log.warning(msg);
} else {
this.v3Utils.log.notice(msg);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Doing a string search is probably fine for this PR, but I think it's best for the caller to decide between .warning() and .notice().

Was there a similar this.serverless.cli.warn() in v2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, there wasn't a cli.warn previously and only with this new logging mechanism we've introduced proper logging levels.

I fully agree that called should be responsible for deciding the log level of the message, but on the other hand I wanted to keep the changes as small as possible. What do you think makes more sense from the perspective of the plugin here?

Copy link
Owner

Choose a reason for hiding this comment

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

I'm happy to do a refactor after merging this PR in. Thanks!

} else {
this.serverless.cli.log(msg);
}
}
}

Expand Down
134 changes: 100 additions & 34 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,24 @@ describe('ServerlessPlugin', function () {
},
};
this.options = {};
this.v3Utils = {
log: {
notice: this.sandbox.stub(),
warning: this.sandbox.stub(),
},
};

this.createPlugin = () =>
new this.ServerlessPlugin(this.serverless, this.options);
this.createPlugin = ({ withV3Utils } = {}) => {
if (withV3Utils) {
return new this.ServerlessPlugin(
this.serverless,
this.options,
this.v3Utils,
);
} else {
return new this.ServerlessPlugin(this.serverless, this.options);
}
};
});

afterEach(function () {
Expand Down Expand Up @@ -82,22 +97,45 @@ describe('ServerlessPlugin', function () {
});

describe('log()', function () {
it('logs by default', function () {
const msg = 'msg';
this.createPlugin().log(msg);
this.serverless.cli.log.should.be.calledWith(msg);
});
[true, false].forEach((withV3Utils) => {
describe(JSON.stringify({ withV3Utils }), function () {
it('logs by default', function () {
const msg = 'msg';
this.createPlugin({ withV3Utils }).log(msg);
if (withV3Utils) {
this.serverless.cli.log.should.not.be.called;
this.v3Utils.log.notice.should.be.calledWith(msg);
} else {
this.serverless.cli.log.should.be.calledWith(msg);
this.v3Utils.log.notice.should.not.be.called;
}
});

it('does nothing if logging is disabled', function () {
this.serverless.service.custom = {
dotenv: {
logging: false,
},
};
it('logs by default and uses warning method if needed', function () {
const msg = 'WARNING: msg';
this.createPlugin({ withV3Utils }).log(msg);

if (withV3Utils) {
this.serverless.cli.log.should.not.be.called;
this.v3Utils.log.warning.should.be.calledWith(msg);
} else {
this.serverless.cli.log.should.be.calledWith(msg);
this.v3Utils.log.notice.should.not.be.called;
}
});

this.createPlugin().log('msg');
it('does nothing if logging is disabled', function () {
this.serverless.service.custom = {
dotenv: {
logging: false,
},
};

this.serverless.cli.log.should.not.be.called;
this.createPlugin({ withV3Utils }).log('msg');
this.serverless.cli.log.should.not.be.called;
this.v3Utils.log.notice.should.not.be.called;
});
});
});
});

Expand Down Expand Up @@ -148,21 +186,32 @@ describe('ServerlessPlugin', function () {
this.createPlugin().resolveEnvFileNames('env').should.deep.equal(path);
});

it('logs an error if basePath is also set', function () {
const path = '.env.unittest';
this.serverless.service.custom = {
dotenv: {
basePath: 'base/path/',
path,
},
};

this.createPlugin()
.resolveEnvFileNames('env')
.should.deep.equal([path]);
this.serverless.cli.log.should.have.been.calledWith(
sinon.match(/basePath/),
);
[true, false].forEach((withV3Utils) => {
describe(JSON.stringify({ withV3Utils }), function () {
it('logs an error if basePath is also set', function () {
const path = '.env.unittest';
this.serverless.service.custom = {
dotenv: {
basePath: 'base/path/',
path,
},
};

this.createPlugin({ withV3Utils })
.resolveEnvFileNames('env')
.should.deep.equal([path]);

if (withV3Utils) {
this.v3Utils.log.warning.should.have.been.calledWith(
sinon.match(/basePath/),
);
} else {
this.serverless.cli.log.should.have.been.calledWith(
sinon.match(/basePath/),
);
}
});
});
});
});

Expand Down Expand Up @@ -334,14 +383,31 @@ describe('ServerlessPlugin', function () {
});
});

it('logs an error if no .env files are required and none are found', function () {
const log = this.sandbox.stub(this.ServerlessPlugin.prototype, 'log');
[true, false].forEach((withV3Utils) => {
describe(JSON.stringify({ withV3Utils }), function () {
it('logs an error if no .env files are required and none are found', function () {
const resolveEnvFileNames = this.setupResolveEnvFileNames();
resolveEnvFileNames.withArgs(this.env).returns([]);

this.createPlugin({ withV3Utils });

const expectedMsg = 'DOTENV: Could not find .env file.';
if (withV3Utils) {
this.v3Utils.log.notice.should.have.been.calledWith(expectedMsg);
} else {
this.serverless.cli.log.should.have.been.calledWith(expectedMsg);
}
});
});
});
it('logs an error with V3Utils if no .env files are required and none are found', function () {
const resolveEnvFileNames = this.setupResolveEnvFileNames();
resolveEnvFileNames.withArgs(this.env).returns([]);

this.createPlugin();
log.should.have.been.calledWith('DOTENV: Could not find .env file.');
this.createPlugin({ withV3Utils: true });
this.v3Utils.log.notice.should.have.been.calledWith(
'DOTENV: Could not find .env file.',
);
});

it('throws an error if no .env files are found but at least one is required', function () {
Expand Down