Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Handle incorrect keys to loggy #1112

Merged
merged 2 commits into from
Jul 17, 2019
Merged

Handle incorrect keys to loggy #1112

merged 2 commits into from
Jul 17, 2019

Conversation

spalladino
Copy link
Contributor

When calling Loggy.success with a non-existing key, the reference would not be found, and the call to path.basename in Loggy._log would fail (since its parameter was null). This commit:

  • Catches any exceptions in _log, and displays an error (allowing to continue)
  • Adds a testing mode to the logger, which is silent but executes logic, and throws an exception upon issues (only to be used in the CLI and lib tests)
  • Fixes incorrect or missing keys in calls to the logger

Note: we should cherry pick this one onto 2.4

When calling Loggy.success with a non-existing key, the reference would not be found, and the call to path.basename in Loggy._log would fail (since its parameter was null). This commit:

- Catches any exceptions in _log, and displays an error (allowing to continue)
- Adds a testing mode to the logger, which is silent but executes logic, and throws an exception upon issues (only to be used in the CLI and lib tests)
- Fixes incorrect or missing keys in calls to the logger
@@ -233,7 +233,7 @@ export default class NetworkController {
Loggy.spin(__filename, '_uploadSolidityLib', `upload-solidity-lib${libName}`, `Uploading ${libName} library`);
const libInstance = await this.project.setImplementation(libClass, libName);
this.networkFile.addSolidityLib(libName, libInstance);
Loggy.succeed(`upload-solidity-lob${libName}`, `${libName} library uploaded`);
Loggy.succeed(`upload-solidity-lib${libName}`, `${libName} library uploaded`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch!

describe('Loggger', function() {
afterEach('restore logger', function() {
Loggy.silent(true);
describe('Logger', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Loggy.isSilent.should.be.true;
Loggy.isVerbose.should.be.false;
});

it(`has a 'logs' object initialized`, function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This test seems to be failing because Loggy.logs is being filled by some other test. I believe it's not even necessary to have it, as it's only testing a variable assignment rather than a behavior.

@jbcarpanelli jbcarpanelli added the status:ready-to-merge Order mergify to merge label Jul 17, 2019
@mergify mergify bot merged commit 63c1535 into master Jul 17, 2019
@mergify mergify bot deleted the fix/logger-keys branch July 17, 2019 19:14
spalladino added a commit that referenced this pull request Jul 17, 2019
* Handle incorrect keys to loggy

When calling Loggy.success with a non-existing key, the reference would not be found, and the call to path.basename in Loggy._log would fail (since its parameter was null). This commit:

- Catches any exceptions in _log, and displays an error (allowing to continue)
- Adds a testing mode to the logger, which is silent but executes logic, and throws an exception upon issues (only to be used in the CLI and lib tests)
- Fixes incorrect or missing keys in calls to the logger

* Remove failing test
jbcarpanelli pushed a commit that referenced this pull request Jul 18, 2019
* Do not fail if contracts folder is missing (#1107)

Fixes #1068

* Handle incorrect keys to loggy (#1112)

* Handle incorrect keys to loggy

When calling Loggy.success with a non-existing key, the reference would not be found, and the call to path.basename in Loggy._log would fail (since its parameter was null). This commit:

- Catches any exceptions in _log, and displays an error (allowing to continue)
- Adds a testing mode to the logger, which is silent but executes logic, and throws an exception upon issues (only to be used in the CLI and lib tests)
- Fixes incorrect or missing keys in calls to the logger

* Remove failing test

* Use require.resolve to lookup contracts in deps (#1110)

* Use chai expect throw syntax in Dependency tests

* Use require.resolve to lookup contracts in deps

Change lookup in both Dependency and Contracts classes. Instead of looking for just node_modules in the current folder, it uses require.resolve (relative to the current workdir) to look for the dependency.

Fixes #1076

* Fix tests

* Add changelogs

* Reset circle cache
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:ready-to-merge Order mergify to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants