test: add a test for re-running m0002010

Author: ctron

common/src/db/mod.rs

@@ -103,6 +103,10 @@ impl Database {
     pub fn name(&self) -> &str {
         &self.name
     }
+
+    pub fn into_connection(self) -> DatabaseConnection {
+        self.db
+    }
 }
 
 impl Deref for Database {

migration/src/bin/data.rs

@@ -1,7 +1,7 @@
 use clap::Parser;
 use migration::{
     Migrator,
-    data::{Direction, MigratorWithData, Options, Runner},
+    data::{Database, Direction, MigratorWithData, Options, Runner},
 };
 use trustify_module_storage::config::StorageConfig;
 
@@ -85,10 +85,12 @@ impl Run {
             direction,
             storage,
             migrations: self.migrations,
-            database_url: self
-                .database_url
-                .expect("Environment variable 'DATABASE_URL' not set"),
-            database_schema: self.database_schema,
+            database: Database::Config {
+                url: self
+                    .database_url
+                    .expect("Environment variable 'DATABASE_URL' not set"),
+                schema: self.database_schema,
+            },
             options: self.options,
         }
         .run::<Migrator>()

migration/src/data/migration.rs

@@ -21,6 +21,7 @@ static OPTIONS: LazyLock<Options> = LazyLock::new(init_options);
 
 task_local! {
     static TEST_STORAGE: DispatchBackend;
+    static TEST_OPTIONS: Options;
 }
 
 #[allow(clippy::expect_used)]
@@ -46,9 +47,13 @@ impl MigrationWithData {
             .try_with(|s| s.clone())
             .unwrap_or_else(|_| STORAGE.clone());
 
+        let options = TEST_OPTIONS
+            .try_with(|o| o.clone())
+            .unwrap_or_else(|_| OPTIONS.clone());
+
         Self {
             storage,
-            options: OPTIONS.clone(),
+            options,
             migration,
         }
     }
@@ -57,11 +62,19 @@ impl MigrationWithData {
     ///
     /// This will, for the duration of the call, initialize the migrator with the provided storage
     /// backend.
-    pub async fn run_with_test_storage<F>(storage: impl Into<DispatchBackend>, f: F) -> F::Output
+    pub async fn run_with_test<F>(
+        storage: impl Into<DispatchBackend>,
+        options: impl Into<Options>,
+        f: F,
+    ) -> F::Output
     where
         F: Future,
     {
-        TEST_STORAGE.scope(storage.into(), f).await
+        TEST_STORAGE
+            .scope(storage.into(), async {
+                TEST_OPTIONS.scope(options.into(), f).await
+            })
+            .await
     }
 }
 

migration/src/data/mod.rs

@@ -73,6 +73,12 @@ impl Default for Options {
     }
 }
 
+impl From<()> for Options {
+    fn from(_: ()) -> Self {
+        Self::default()
+    }
+}
+
 impl Options {
     pub fn should_skip(&self, name: &str) -> bool {
         if self.skip_all {

migration/src/data/run.rs

@@ -1,9 +1,8 @@
 use crate::data::{MigratorWithData, Options, SchemaDataManager};
 use anyhow::bail;
-use sea_orm::{ConnectOptions, Database};
+use sea_orm::ConnectOptions;
 use sea_orm_migration::{IntoSchemaManagerConnection, SchemaManager};
-use std::collections::HashMap;
-use std::time::SystemTime;
+use std::{collections::HashMap, time::SystemTime};
 use trustify_module_storage::service::dispatch::DispatchBackend;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, clap::ValueEnum)]
@@ -14,14 +13,18 @@ pub enum Direction {
 }
 
 pub struct Runner {
-    pub database_url: String,
-    pub database_schema: Option<String>,
+    pub database: Database,
     pub storage: DispatchBackend,
     pub direction: Direction,
     pub migrations: Vec<String>,
     pub options: Options,
 }
 
+pub enum Database {
+    Config { url: String, schema: Option<String> },
+    Provided(sea_orm::DatabaseConnection),
+}
+
 impl Runner {
     pub async fn run<M: MigratorWithData>(self) -> anyhow::Result<()> {
         let migrations = M::data_migrations()
@@ -38,15 +41,20 @@ impl Runner {
             running.push(migration);
         }
 
-        let schema = self.database_schema.unwrap_or_else(|| "public".to_owned());
+        let database = match self.database {
+            Database::Config { url, schema } => {
+                let schema = schema.unwrap_or_else(|| "public".to_owned());
 
-        let connect_options = ConnectOptions::new(self.database_url)
-            .set_schema_search_path(schema)
-            .to_owned();
+                let connect_options = ConnectOptions::new(url)
+                    .set_schema_search_path(schema)
+                    .to_owned();
 
-        let db = Database::connect(connect_options).await?;
+                sea_orm::Database::connect(connect_options).await?
+            }
+            Database::Provided(database) => database,
+        };
 
-        let manager = SchemaManager::new(db.into_schema_manager_connection());
+        let manager = SchemaManager::new(database.into_schema_manager_connection());
         let manager = SchemaDataManager::new(&manager, &self.storage, &self.options);
 
         for run in running {

migration/tests/data/m0002010.rs

@@ -0,0 +1,44 @@
+use crate::MigratorTest;
+use migration::Migrator;
+use migration::data::{Database, Direction, MigrationWithData, Options, Runner};
+use sea_orm_migration::MigratorTrait;
+use test_context::test_context;
+use test_log::test;
+use trustify_test_context::{TrustifyMigrationContext, commit, ctx::DumpId};
+
+commit!(Commit("8c6ad23172e66a6c923dcc8f702e6125a8d48723"));
+
+#[test_context(TrustifyMigrationContext<Commit>)]
+#[test(tokio::test)]
+async fn examples(
+    ctx: &TrustifyMigrationContext<Commit>, /* commit previous to this PR */
+) -> Result<(), anyhow::Error> {
+    let migrations = vec!["m0002010_add_advisory_scores".into()];
+
+    // first run the data migration
+    Runner {
+        direction: Direction::Up,
+        storage: ctx.storage.clone().into(),
+        migrations: migrations.clone(),
+        database: Database::Provided(ctx.db.clone().into_connection()),
+        options: Default::default(),
+    }
+    .run::<Migrator>()
+    .await?;
+
+    // now run the migrations, but skip the already run migration
+
+    MigrationWithData::run_with_test(
+        ctx.storage.clone(),
+        Options {
+            skip: migrations,
+            ..Default::default()
+        },
+        async { MigratorTest::up(&ctx.db, None).await },
+    )
+    .await?;
+
+    // done
+
+    Ok(())
+}

migration/tests/data.rs renamed to migration/tests/data/main.rs

@@ -1,3 +1,5 @@
+mod m0002010;
+
 use migration::{
     Migrator, MigratorExt,
     data::{MigrationWithData, Migrations},
@@ -116,7 +118,7 @@ impl MigratorTrait for MigratorTest {
 #[test_context(TrustifyMigrationContext)]
 #[test(tokio::test)]
 async fn examples(ctx: &TrustifyMigrationContext) -> Result<(), anyhow::Error> {
-    MigrationWithData::run_with_test_storage(ctx.storage.clone(), async {
+    MigrationWithData::run_with_test(ctx.storage.clone(), (), async {
         MigratorTest::up(&ctx.db, None).await
     })
     .await?;

migration/tests/previous.rs

@@ -11,7 +11,7 @@ async fn from_previous(ctx: &TrustifyMigrationContext) -> Result<(), anyhow::Err
     // We automatically start with a database imported from the previous commit.
     // But we haven't migrated to the most recent schema so far. That's done by the next step.
 
-    MigrationWithData::run_with_test_storage(ctx.storage.clone(), async {
+    MigrationWithData::run_with_test(ctx.storage.clone(), (), async {
         Database(&ctx.db).migrate().await
     })
     .await?;

test-context/src/ctx/migration.rs

@@ -1,26 +1,58 @@
 use crate::{TrustifyTestContext, migration::Migration};
 use anyhow::Context;
+use std::borrow::Cow;
+use std::marker::PhantomData;
 use std::ops::Deref;
 use tar::Archive;
 use test_context::AsyncTestContext;
 use trustify_db::embedded::{Options, Source, default_settings};
 use trustify_module_storage::service::fs::FileSystemBackend;
 
+#[macro_export]
+macro_rules! commit {
+    ($t:ident($id:literal)) => {
+        pub struct $t;
+
+        impl DumpId for $t {
+            fn dump_id() -> Option<&'static str> {
+                Some($id)
+            }
+        }
+    };
+}
+
+pub trait DumpId {
+    fn dump_id() -> Option<&'static str>;
+}
+
+impl DumpId for () {
+    fn dump_id() -> Option<&'static str> {
+        None
+    }
+}
+
 /// Creates a database and imports the previous DB and storage dump.
-pub struct TrustifyMigrationContext(pub(crate) TrustifyTestContext);
+pub struct TrustifyMigrationContext<ID: DumpId = ()>(
+    pub(crate) TrustifyTestContext,
+    PhantomData<ID>,
+);
 
-impl Deref for TrustifyMigrationContext {
+impl<ID: DumpId> Deref for TrustifyMigrationContext<ID> {
     type Target = TrustifyTestContext;
 
     fn deref(&self) -> &Self::Target {
         &self.0
     }
 }
 
-impl TrustifyMigrationContext {
+impl<ID: DumpId> TrustifyMigrationContext<ID> {
     pub async fn new() -> anyhow::Result<Self> {
         let migration = Migration::new().expect("failed to create migration manager");
-        let base = migration.provide().await?;
+        let id: Cow<'static, str> = match ID::dump_id() {
+            Some(id) => format!("commit-{id}").into(),
+            None => "latest".into(),
+        };
+        let base = migration.provide(&id).await?;
 
         // create storage
 
@@ -50,13 +82,14 @@ impl TrustifyMigrationContext {
 
         Ok(Self(
             TrustifyTestContext::new(db, storage, tmp, postgresql).await,
+            Default::default(),
         ))
     }
 }
 
-impl AsyncTestContext for TrustifyMigrationContext {
+impl<ID: DumpId> AsyncTestContext for TrustifyMigrationContext<ID> {
     async fn setup() -> Self {
-        TrustifyMigrationContext::new()
+        Self::new()
             .await
             .expect("failed to create migration context")
     }

test-context/src/migration.rs

@@ -68,8 +68,8 @@ impl Migration {
     /// Provide the base dump path, for this branch.
     ///
     /// This may include downloading content from S3.
-    pub async fn provide(&self) -> anyhow::Result<PathBuf> {
-        let base = self.base.join(&self.branch);
+    pub async fn provide(&self, id: &str) -> anyhow::Result<PathBuf> {
+        let base = self.base.join(&self.branch).join(id);
 
         log::info!("branch base path: '{}'", base.display());
 
@@ -101,6 +101,7 @@ impl Migration {
                 &self.bucket,
                 &self.region,
                 &self.branch,
+                id,
                 files,
             )
             .await?
@@ -297,6 +298,7 @@ async fn download_artifacts(
     bucket: &str,
     region: &str,
     branch: &str,
+    commit: &str,
     files: impl IntoIterator<Item = impl AsRef<str>>,
 ) -> anyhow::Result<()> {
     let base = base.as_ref();
@@ -305,10 +307,7 @@ async fn download_artifacts(
         let file = file.as_ref();
         vec![file.to_string(), format!("{file}.sha256")]
     }) {
-        let url = format!(
-            "https://{}.s3.{}.amazonaws.com/{}/latest/{}",
-            bucket, region, branch, file
-        );
+        let url = format!("https://{bucket}.s3.{region}.amazonaws.com/{branch}/{commit}/{file}",);
 
         log::info!("downloading file: '{url}'");