fix: sqlx mig add behavior and tests

Author: abonander

sqlx-cli/src/opt.rs

@@ -435,11 +435,32 @@ impl AddMigrationOpts {
 
         match (self.timestamp, self.sequential, default_versioning) {
             (true, false, _) | (false, false, DefaultVersioning::Timestamp) => next_timestamp(),
-            (false, true, _) | (false, false, DefaultVersioning::Sequential) => {
-                next_sequential(migrator).unwrap_or_else(|| fmt_sequential(1))
-            }
+            (false, true, _) | (false, false, DefaultVersioning::Sequential) => fmt_sequential(
+                migrator
+                    .migrations
+                    .last()
+                    .map_or(1, |migration| migration.version + 1),
+            ),
             (false, false, DefaultVersioning::Inferred) => {
-                next_sequential(migrator).unwrap_or_else(next_timestamp)
+                migrator
+                    .migrations
+                    .rchunks(2)
+                    .next()
+                    .and_then(|migrations| {
+                        match migrations {
+                            [previous, latest] => {
+                                // If the latest two versions differ by 1, infer sequential.
+                                (latest.version - previous.version == 1)
+                                    .then_some(latest.version + 1)
+                            }
+                            [latest] => {
+                                // If only one migration exists and its version is 0 or 1, infer sequential
+                                matches!(latest.version, 0 | 1).then_some(latest.version + 1)
+                            }
+                            _ => unreachable!(),
+                        }
+                    })
+                    .map_or_else(next_timestamp, fmt_sequential)
             }
             (true, true, _) => unreachable!("BUG: Clap should have rejected this case"),
         }
@@ -450,28 +471,6 @@ fn next_timestamp() -> String {
     Utc::now().format("%Y%m%d%H%M%S").to_string()
 }
 
-fn next_sequential(migrator: &Migrator) -> Option<String> {
-    let next_version = migrator
-        .migrations
-        .rchunks(2)
-        .next()
-        .and_then(|migrations| {
-            match migrations {
-                [previous, latest] => {
-                    // If the latest two versions differ by 1, infer sequential.
-                    (latest.version - previous.version == 1).then_some(latest.version + 1)
-                }
-                [latest] => {
-                    // If only one migration exists and its version is 0 or 1, infer sequential
-                    matches!(latest.version, 0 | 1).then_some(latest.version + 1)
-                }
-                _ => unreachable!(),
-            }
-        });
-
-    next_version.map(fmt_sequential)
-}
-
 fn fmt_sequential(version: i64) -> String {
     format!("{version:04}")
 }

sqlx-cli/tests/add.rs

@@ -34,11 +34,6 @@ impl PartialOrd<Self> for FileName {
 
 impl FileName {
     fn assert_is_timestamp(&self) {
-        //if the library is still used in 2050, this will need bumping ^^
-        assert!(
-            self.id < 20500101000000,
-            "{self:?} is too high for a timestamp"
-        );
         assert!(
             self.id > 20200101000000,
             "{self:?} is too low for a timestamp"
@@ -74,10 +69,12 @@ fn add_migration_sequential() -> anyhow::Result<()> {
             .run("hello world1", false, false, true, true)?
             .run("hello world2", true, false, true, true)?
             .fs_output()?;
-        assert_eq!(files.len(), 2);
-        files.assert_is_not_reversible();
+        assert_eq!(files.len(), 3);
         assert_eq!(files.0[0].id, 1);
         assert_eq!(files.0[1].id, 2);
+        assert_eq!(files.0[1].suffix, "down.sql");
+        assert_eq!(files.0[2].id, 2);
+        assert_eq!(files.0[2].suffix, "up.sql");
     }
     Ok(())
 }
@@ -126,11 +123,11 @@ fn add_migration_timestamp() -> anyhow::Result<()> {
             .run("hello world1", false, true, false, true)?
             .run("hello world2", true, false, true, true)?
             .fs_output()?;
-        assert_eq!(files.len(), 2);
-        files.assert_is_not_reversible();
+        assert_eq!(files.len(), 3);
         files.0[0].assert_is_timestamp();
         // sequential -> timestamp is one way
         files.0[1].assert_is_timestamp();
+        files.0[2].assert_is_timestamp();
     }
     Ok(())
 }