Merge pull request #1905 from trufflesuite/update-skip-dry-run

Reformat migrations and implement cleaner dry run behavior

packages/truffle-config/index.js

@@ -248,20 +248,6 @@ function Config(truffle_directory, working_directory, network) {
           "Don't set config.timeoutBlocks directly. Instead, set config.networks and then config.networks[<network name>].timeoutBlocks"
         );
       }
-    },
-    skipDryRun: {
-      get: function() {
-        try {
-          return self.network_config.skipDryRun;
-        } catch (e) {
-          return false;
-        }
-      },
-      set: function() {
-        throw new Error(
-          "Don't set config.skipDryRun directly. Instead, set config.networks and then config.networks[<network name>].skipDryRun"
-        );
-      }
     }
   };
 
@@ -327,16 +313,17 @@ Config.prototype.with = function(obj) {
 };
 
 Config.prototype.merge = function(obj) {
-  var self = this;
-  var clone = this.normalize(obj);
+  let clone = this.normalize(obj);
 
   // Only set keys for values that don't throw.
-  Object.keys(obj).forEach(function(key) {
+  const propertyNames = Object.keys(obj);
+
+  propertyNames.forEach(key => {
     try {
-      if (typeof clone[key] === "object" && self._deepCopy.includes(key)) {
-        self[key] = _.merge(self[key], clone[key]);
+      if (typeof clone[key] === "object" && this._deepCopy.includes(key)) {
+        this[key] = _.merge(this[key], clone[key]);
       } else {
-        self[key] = clone[key];
+        this[key] = clone[key];
       }
     } catch (e) {
       // Do nothing.
@@ -387,14 +374,14 @@ Config.detect = (options = {}, filename) => {
 };
 
 Config.load = function(file, options) {
-  var config = new Config();
+  const config = new Config();
 
   config.working_directory = path.dirname(path.resolve(file));
 
   // The require-nocache module used to do this for us, but
   // it doesn't bundle very well. So we've pulled it out ourselves.
   delete require.cache[Module._resolveFilename(file, module)];
-  var static_config = originalrequire(file);
+  const static_config = originalrequire(file);
 
   config.merge(static_config);
   config.merge(options);

packages/truffle-core/lib/commands/migrate.js

@@ -1,4 +1,4 @@
-var command = {
+const command = {
   command: "migrate",
   description: "Run migrations to deploy contracts",
   builder: {
@@ -16,6 +16,11 @@ var command = {
       type: "boolean",
       default: false
     },
+    "skip-dry-run": {
+      describe: "Skip the test or 'dry run' migrations",
+      type: "boolean",
+      default: false
+    },
     "f": {
       describe: "Specify a migration number to run from",
       type: "number"
@@ -85,18 +90,10 @@ var command = {
       }
     ]
   },
-  run: function(options, done) {
-    var Config = require("truffle-config");
-    var Contracts = require("truffle-workflow-compile");
-    var Resolver = require("truffle-resolver");
-    var Artifactor = require("truffle-artifactor");
-    var Migrate = require("truffle-migrate");
-    var Environment = require("../environment");
-    var temp = require("temp");
-    var copy = require("../copy");
 
+  determineDryRunSettings: (config, options) => {
     // Source: ethereum.stackexchange.com/questions/17051
-    var networkWhitelist = [
+    const networkWhitelist = [
       1, // Mainnet (ETH & ETC)
       2, // Morden (ETC)
       3, // Ropsten
@@ -110,6 +107,70 @@ var command = {
       61717561 // Aquachain
     ];
 
+    let dryRunOnly, skipDryRun;
+    const networkSettingsInConfig = config.networks[config.network];
+    if (networkSettingsInConfig) {
+      dryRunOnly =
+        options.dryRun === true ||
+        networkSettingsInConfig.dryRun === true ||
+        networkSettingsInConfig["dry-run"] === true;
+      skipDryRun =
+        options.skipDryRun === true ||
+        networkSettingsInConfig.skipDryRun === true ||
+        networkSettingsInConfig["skip-dry-run"] === true;
+    } else {
+      dryRunOnly = options.dryRun === true;
+      skipDryRun = options.skipDryRun === true;
+    }
+    const production =
+      networkWhitelist.includes(parseInt(config.network_id)) ||
+      config.production;
+    const dryRunAndMigrations = production && !skipDryRun;
+    return { dryRunOnly, dryRunAndMigrations };
+  },
+
+  run: function(options, done) {
+    const Artifactor = require("truffle-artifactor");
+    const Resolver = require("truffle-resolver");
+    const Migrate = require("truffle-migrate");
+    const Contracts = require("truffle-workflow-compile");
+    const Environment = require("../environment");
+    const Config = require("truffle-config");
+    const temp = require("temp");
+    const copy = require("../copy");
+
+    const conf = Config.detect(options);
+
+    async function executePostDryRunMigration(buildDir, options, done) {
+      const Artifactor = require("truffle-artifactor");
+      const Resolver = require("truffle-resolver");
+      const Migrate = require("truffle-migrate");
+      const Config = require("truffle-config");
+      let accept = true;
+
+      if (options.interactive) {
+        accept = await Migrate.acceptDryRun();
+      }
+
+      if (accept) {
+        const environment = require("../environment");
+        const config = Config.detect(options);
+
+        config.contracts_build_directory = buildDir;
+        config.artifactor = new Artifactor(buildDir);
+        config.resolver = new Resolver(config);
+
+        environment.detect(config, err => {
+          if (err) return done(err);
+
+          config.dryRun = false;
+          runMigrations(config, done);
+        });
+      } else {
+        done();
+      }
+    }
+
     function setupDryRunEnvironmentThenRunMigrations(config) {
       return new Promise((resolve, reject) => {
         Environment.fork(config, function(err) {
@@ -148,91 +209,73 @@ var command = {
       });
     }
 
-    function runMigrations(config, callback) {
+    async function runMigrations(config, callback) {
       Migrate.launchReporter();
 
       if (options.f) {
-        Migrate.runFrom(options.f, config, done);
+        try {
+          await Migrate.runFrom(options.f, config);
+          done();
+        } catch (error) {
+          done(error);
+        }
       } else {
-        Migrate.needsMigrating(config, function(err, needsMigrating) {
-          if (err) return callback(err);
+        try {
+          const needsMigrating = await Migrate.needsMigrating(config);
 
           if (needsMigrating) {
             Migrate.run(config, callback);
           } else {
             config.logger.log("Network up to date.");
             callback();
           }
-        });
+        } catch (error) {
+          callback(error);
+        }
       }
     }
 
-    async function executePostDryRunMigration(buildDir) {
-      let accept = true;
+    const detectCallback = async function(error) {
+      if (error) return done(error);
 
-      if (options.interactive) {
-        accept = await Migrate.acceptDryRun();
-      }
+      const { dryRunOnly, dryRunAndMigrations } = this.determineDryRunSettings(
+        conf,
+        options
+      );
 
-      if (accept) {
-        const environment = require("../environment");
-        const NewConfig = require("truffle-config");
-        const config = NewConfig.detect(options);
+      if (dryRunOnly) {
+        try {
+          conf.dryRun = true;
+          await setupDryRunEnvironmentThenRunMigrations(conf);
+          done();
+        } catch (err) {
+          done(err);
+        }
+      } else if (dryRunAndMigrations) {
+        const currentBuild = conf.contracts_build_directory;
+        conf.dryRun = true;
 
-        config.contracts_build_directory = buildDir;
-        config.artifactor = new Artifactor(buildDir);
-        config.resolver = new Resolver(config);
+        try {
+          await setupDryRunEnvironmentThenRunMigrations(conf);
+        } catch (err) {
+          return done(err);
+        }
 
-        environment.detect(config, err => {
-          if (err) return done(err);
+        executePostDryRunMigration(currentBuild, options, done);
 
-          config.dryRun = false;
-          runMigrations(config, done);
-        });
+        // Development
       } else {
-        done();
+        await runMigrations(conf, done);
       }
-    }
-
-    const conf = Config.detect(options);
-
-    Contracts.compile(conf, function(err) {
-      if (err) return done(err);
+    };
 
-      Environment.detect(conf, async function(err) {
-        if (err) return done(err);
+    const compileCallback = function(error) {
+      if (error) return done(error);
 
-        const dryRunOnly = options.dryRun === true;
-        const production =
-          networkWhitelist.includes(parseInt(conf.network_id)) ||
-          conf.production;
-        const dryRunAndMigration = production && !conf.skipDryRun;
+      Environment.detect(conf, detectCallback.bind(this));
+    };
 
-        if (dryRunOnly) {
-          try {
-            await setupDryRunEnvironmentThenRunMigrations(conf);
-            done();
-          } catch (err) {
-            done(err);
-          }
-        } else if (dryRunAndMigration) {
-          const currentBuild = conf.contracts_build_directory;
-          conf.dryRun = true;
-
-          try {
-            await setupDryRunEnvironmentThenRunMigrations(conf);
-          } catch (err) {
-            return done(err);
-          }
-
-          executePostDryRunMigration(currentBuild);
-
-          // Development
-        } else {
-          runMigrations(conf, done);
-        }
-      });
-    });
+    Contracts.compile(conf, compileCallback.bind(this));
   }
 };
 

packages/truffle-core/test/migrate.js

@@ -219,43 +219,33 @@ describe("migrate", function() {
     });
   });
 
-  it("should ignore files that don't start with a number", function(done) {
+  it("should ignore files that don't start with a number", () => {
     fs.writeFileSync(
       path.join(config.migrations_directory, "~2_deploy_contracts.js"),
       "module.exports = function() {};",
       "utf8"
     );
 
-    Migrate.assemble(config, function(err, migrations) {
-      if (err) return done(err);
-
-      assert.equal(
-        migrations.length,
-        2,
-        "~2_deploy_contracts.js should have been ignored!"
-      );
-
-      done();
-    });
+    const migrations = Migrate.assemble(config);
+    assert.equal(
+      migrations.length,
+      2,
+      "~2_deploy_contracts.js should have been ignored!"
+    );
   });
 
-  it("should ignore non-js extensions", function(done) {
+  it("should ignore non-js extensions", () => {
     fs.writeFileSync(
       path.join(config.migrations_directory, "2_deploy_contracts.js~"),
       "module.exports = function() {};",
       "utf8"
     );
 
-    Migrate.assemble(config, function(err, migrations) {
-      if (err) return done(err);
-
-      assert.equal(
-        migrations.length,
-        2,
-        "2_deploy_contracts.js~ should have been ignored!"
-      );
-
-      done();
-    });
+    const migrations = Migrate.assemble(config);
+    assert.equal(
+      migrations.length,
+      2,
+      "2_deploy_contracts.js~ should have been ignored!"
+    );
   });
 });