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
OpenZeppelin · Jul 18, 2019 · fc33b7b · fc33b7b
1 parent faa6906
commit fc33b7b
Show file tree

Hide file tree

Showing 10 changed files with 132 additions and 115 deletions.
diff --git a/package.json b/package.json
@@ -6,6 +6,7 @@
   },
   "devDependencies": {
     "lerna": "~3.13.1",
-    "typescript": "^3.2.2"
+    "typescript": "^3.2.2",
+    "mock-stdlib-root": "file:./packages/cli/test/mocks/mock-stdlib"
   }
 }
diff --git a/packages/cli/package-lock.json b/packages/cli/package-lock.json
diff --git a/packages/cli/src/models/dependency/Dependency.ts b/packages/cli/src/models/dependency/Dependency.ts
@@ -57,13 +57,21 @@ export default class Dependency {
     }
   }
 
-  public static hasDependenciesForDeploy(network: string): boolean {
-    const dependencies = ZosPackageFile.getLinkedDependencies() || [];
+  public static hasDependenciesForDeploy(
+    network: string,
+    packageFilename = 'zos.json',
+    networkFilename: string = undefined,
+  ): boolean {
+    const dependencies =
+      ZosPackageFile.getLinkedDependencies(packageFilename) || [];
     const networkDependencies =
-      ZosNetworkFile.getDependencies(`zos.${network}.json`) || {};
+      ZosNetworkFile.getDependencies(
+        networkFilename || `zos.${network}.json`,
+      ) || {};
     const hasDependenciesForDeploy = dependencies.find(depNameAndVersion => {
       const [name, version] = depNameAndVersion.split('@');
-      const networkFilePath = `node_modules/${name}/zos.${network}.json`;
+      const dependency = new Dependency(name);
+      const networkFilePath = dependency._getNetworkFilePath(network);
       const projectDependency = networkDependencies[name];
       const satisfiesVersion =
         projectDependency &&
@@ -151,15 +159,16 @@ export default class Dependency {
 
   public getPackageFile(): ZosPackageFile | never {
     if (!this._packageFile) {
-      const filename = `node_modules/${this.name}/zos.json`;
-      if (!fs.exists(filename)) {
+      try {
+        const filename = require.resolve(`${this.name}/zos.json`, {
+          paths: [process.cwd()],
+        });
+        this._packageFile = new ZosPackageFile(filename);
+      } catch (err) {
         throw Error(
-          `Could not find a zos.json file for '${
-            this.name
-          }'. Make sure it is provided by the npm package.`,
+          `Could not find a zos.json file for '${this.name}'. (${err.message})`,
         );
       }
-      this._packageFile = new ZosPackageFile(filename);
     }
     return this._packageFile;
   }
@@ -171,7 +180,7 @@ export default class Dependency {
         throw Error(
           `Could not find a zos file for network '${network}' for '${
             this.name
-          }'`,
+          }'.`,
         );
       }
 
@@ -195,7 +204,13 @@ export default class Dependency {
   }
 
   private _getNetworkFilePath(network: string): string {
-    return `node_modules/${this.name}/zos.${network}.json`;
+    try {
+      return require.resolve(`${this.name}/zos.${network}.json`, {
+        paths: [process.cwd()],
+      });
+    } catch (err) {
+      return null;
+    }
   }
 
   private _validateSatisfiesVersion(

diff --git a/packages/cli/test/models/Dependency.test.js b/packages/cli/test/models/Dependency.test.js
@@ -4,18 +4,9 @@ require('../setup');
 
 import sinon from 'sinon';
 import npm from 'npm-programmatic';
-import { FileSystem as fs } from 'zos-lib';
 import Dependency from '../../src/models/dependency/Dependency';
 
 contract('Dependency', function([_, from]) {
-  const assertErrorMessage = (fn, errorMessage) => {
-    try {
-      fn();
-    } catch (error) {
-      error.message.should.match(errorMessage);
-    }
-  };
-
   describe('static methods', function() {
     describe('#satisfiesVersion', function() {
       it('verifies if requirement satisfies version', function() {
@@ -27,10 +18,9 @@ contract('Dependency', function([_, from]) {
     describe('#fromNameAndVersion', function() {
       describe('with invalid nameAndVersion', function() {
         it('throws error', function() {
-          assertErrorMessage(
-            () => Dependency.fromNameWithVersion('bildts-kcom'),
-            /Could not find a zos.json file/,
-          );
+          expect(
+            () => Dependency.fromNameWithVersion('bildts-kcom')
+          ).to.throw(/Cannot find module/);
         });
       });
 
@@ -62,41 +52,21 @@ contract('Dependency', function([_, from]) {
 
       context('when there are dependencies to deploy', function() {
         it('returns true', function() {
-          const projectPackageFile = fs.parseJsonIfExists(
+          Dependency.hasDependenciesForDeploy(
+            'test', 
             'test/mocks/packages/package-with-multiple-stdlibs.zos.json',
-          );
-          const projectNetworkFile = fs.parseJsonIfExists(
-            'test/mocks/networks/network-with-stdlibs.zos.test.json',
-          );
-          const stubbedParseJsonIfExists = sinon.stub(fs, 'parseJsonIfExists');
-          stubbedParseJsonIfExists
-            .withArgs('zos.json')
-            .returns(projectPackageFile);
-          stubbedParseJsonIfExists
-            .withArgs('zos.test.json')
-            .returns(projectNetworkFile);
-
-          Dependency.hasDependenciesForDeploy('test').should.be.true;
+            'test/mocks/networks/network-with-stdlibs.zos.test.json'
+          ).should.be.true;
         });
       });
 
       context('when all dependencies are already deployed', function() {
         it('returns false', function() {
-          const projectPackageFile = fs.parseJsonIfExists(
+          Dependency.hasDependenciesForDeploy(
+            'test', 
             'test/mocks/packages/package-with-stdlib.zos.json',
-          );
-          const projectNetworkFile = fs.parseJsonIfExists(
-            'test/mocks/networks/network-with-stdlibs.zos.test.json',
-          );
-          const stubbedParseJsonIfExists = sinon.stub(fs, 'parseJsonIfExists');
-          stubbedParseJsonIfExists
-            .withArgs('zos.json')
-            .returns(projectPackageFile);
-          stubbedParseJsonIfExists
-            .withArgs('zos.test.json')
-            .returns(projectNetworkFile);
-
-          Dependency.hasDependenciesForDeploy('test').should.be.false;
+            'test/mocks/networks/network-with-stdlibs.zos.test.json'
+          ).should.be.false;
         });
       });
     });
@@ -105,19 +75,25 @@ contract('Dependency', function([_, from]) {
   describe('#constructor', function() {
     context('with invalid version', function() {
       it('throws an error', function() {
-        assertErrorMessage(
-          () => new Dependency('mock-stdlib', '1.2.0'),
-          /does not match version/,
-        );
+        expect(
+          () => new Dependency('mock-stdlib', '1.2.0')
+        ).to.throw(/does not match version/);
       });
     });
 
     context('with non-existent dependency name', function() {
       it('throws an error', function() {
-        assertErrorMessage(
-          () => new Dependency('bildts-kcom', '1.1.0'),
-          /Could not find a zos.json file/,
-        );
+        expect(
+          () => new Dependency('bildts-kcom', '1.1.0')
+        ).to.throw(/Cannot find module/);
+      });
+    });
+
+    context('with non-ethereum package', function() {
+      it('throws an error', function() {
+        expect(
+          () => new Dependency('chai')
+        ).to.throw(/Cannot find module/);
       });
     });
 
@@ -138,52 +114,54 @@ contract('Dependency', function([_, from]) {
     });
   });
 
-  describe('instance methods', function() {
-    beforeEach(function() {
-      this.dependency = new Dependency('mock-stdlib', '1.1.0');
-      this.txParams = {};
-      this.addresses = {};
-      delete this.dependency._packageFile;
-    });
-
-    describe('#deploy', function() {
-      it('deploys a dependency', async function() {
-        const project = await this.dependency.deploy({ from });
-        const address = await project.getImplementation({
-          contractName: 'Greeter',
+  function testInstanceMethodsFor(libname) {
+    describe(`instance methods for ${libname}`, function() {
+      beforeEach(function() {
+        this.dependency = new Dependency(libname, '1.1.0');
+        this.txParams = {};
+        this.addresses = {};
+        delete this.dependency._projectFile;
+      });
+
+      describe('#deploy', function() {
+        it('deploys a dependency', async function() {
+          const project = await this.dependency.deploy({ from });
+          const address = await project.getImplementation({
+            contractName: 'Greeter',
+          });
+          address.should.be.nonzeroAddress;
         });
-        address.should.be.nonzeroAddress;
       });
-    });
 
-    describe('#getPackageFile', function() {
-      it('generates a package file', function() {
-        const packageFile = this.dependency.getPackageFile();
-        packageFile.should.not.be.null;
-        packageFile.fileName.should.eq('node_modules/mock-stdlib/zos.json');
-        packageFile.version.should.eq('1.1.0');
-        packageFile.contracts.should.include({ Greeter: 'GreeterImpl' });
+      describe('#projectFile', function() {
+        it('generates a package file', function() {
+          const projectFile = this.dependency.getPackageFile();
+          projectFile.should.not.be.null;
+          projectFile.fileName.should.match(/mock-stdlib\/zos\.json$/);
+          projectFile.version.should.eq('1.1.0');
+          projectFile.contracts.should.include({ Greeter: 'GreeterImpl' });
+        });
       });
-    });
 
-    describe('#getNetworkFile', function() {
-      context('for a non-existent network', function() {
-        it('throws an error', function() {
-          assertErrorMessage(
-            () => this.dependency.getNetworkFile('bildts-kcom'),
-            /Could not find a zos file for network/,
-          );
+      describe('#getNetworkFile', function() {
+        context('for a non-existent network', function() {
+          it('throws an error', function() {
+            expect(
+              () => this.dependency.getNetworkFile('bildts-kcom')
+            ).to.throw(/Could not find a zos file/);
+          });
         });
-      });
 
-      context('for an existent network', function() {
-        it('generates network file', function() {
-          const networkFile = this.dependency.getNetworkFile('test');
-          networkFile.fileName.should.eq(
-            'node_modules/mock-stdlib/zos.test.json',
-          );
+        context('for an existent network', function() {
+          it('generates network file', function() {
+            const networkFile = this.dependency.getNetworkFile('test');
+            networkFile.fileName.should.match(/mock-stdlib\/zos\.test\.json$/);
+          });
         });
       });
     });
-  });
+  }
+
+  testInstanceMethodsFor('mock-stdlib');
+  testInstanceMethodsFor('mock-stdlib-root');
 });
diff --git a/packages/cli/test/scripts/unlink.test.js b/packages/cli/test/scripts/unlink.test.js
@@ -28,7 +28,7 @@ contract('unlink script', function() {
         dependencies: [dependencyName],
         packageFile: this.packageFile,
       }).should.be.rejectedWith(
-        `Could not find a zos.json file for '${dependencyName}'. Make sure it is provided by the npm package.`,
+        /Cannot find module/,
       );
     });
   });

diff --git a/packages/lib/src/artifacts/Contracts.ts b/packages/lib/src/artifacts/Contracts.ts
@@ -12,6 +12,7 @@ export default class Contracts {
   private static timeout: number = Contracts.DEFAULT_SYNC_TIMEOUT;
   private static buildDir: string = Contracts.DEFAULT_BUILD_DIR;
   private static contractsDir: string = Contracts.DEFAULT_CONTRACTS_DIR;
+  private static projectRoot: string = null;
   private static artifactDefaults: any = {};
   private static defaultFromAddress: string;
 
@@ -29,6 +30,10 @@ export default class Contracts {
     );
   }
 
+  public static getProjectRoot(): string {
+    return path.resolve(this.projectRoot || process.cwd());
+  }
+
   public static async getDefaultTxParams(): Promise<any> {
     const defaults = { ...Contracts.getArtifactsDefaults() };
     if (!defaults.from) defaults.from = await Contracts.getDefaultFromAddress();
@@ -54,7 +59,10 @@ export default class Contracts {
     dependency: string,
     contractName: string,
   ): string {
-    return `${process.cwd()}/node_modules/${dependency}/build/contracts/${contractName}.json`;
+    return require.resolve(
+      `${dependency}/build/contracts/${contractName}.json`,
+      { paths: [this.getProjectRoot()] },
+    );
   }
 
   public static getFromLocal(contractName: string): Contract {
@@ -98,6 +106,10 @@ export default class Contracts {
     Contracts.contractsDir = dir;
   }
 
+  public static setProjectRoot(dir: string): void {
+    Contracts.projectRoot = dir;
+  }
+
   public static setArtifactsDefaults(defaults: any): void {
     Contracts.artifactDefaults = {
       ...Contracts.getArtifactsDefaults(),

diff --git a/packages/lib/src/project/ProxyAdminProject.ts b/packages/lib/src/project/ProxyAdminProject.ts
@@ -65,10 +65,10 @@ class BaseProxyAdminProject extends BaseSimpleProject {
       initCallData,
     } = await this._setUpgradeParams(proxyAddress, contract, contractParams);
     Loggy.spin(
-      __filename, 
-      'upgradeProxy', 
-      `action-proxy-${pAddress}`, 
-      `Upgrading instance at ${pAddress}`
+      __filename,
+      'upgradeProxy',
+      `action-proxy-${pAddress}`,
+      `Upgrading instance at ${pAddress}`,
     );
     await this.proxyAdmin.upgradeProxy(
       pAddress,

diff --git a/packages/lib/src/project/SimpleProject.ts b/packages/lib/src/project/SimpleProject.ts
@@ -27,10 +27,10 @@ export default class SimpleProject extends BaseSimpleProject {
       initCallData,
     } = await this._setUpgradeParams(proxyAddress, contract, contractParams);
     Loggy.spin(
-      __filename, 
-      'upgradeProxy', 
-      `action-proxy-${pAddress}`, 
-      `Upgrading instance at ${pAddress}`
+      __filename,
+      'upgradeProxy',
+      `action-proxy-${pAddress}`,
+      `Upgrading instance at ${pAddress}`,
     );
     const proxy = Proxy.at(pAddress, this.txParams);
     await proxy.upgradeTo(implementationAddress, initCallData);