simolus3 / drift

Drift is an easy to use, reactive, typesafe persistence library for Dart & Flutter.
https://drift.simonbinder.eu/
MIT License
2.65k stars 372 forks source link

Cannot test migrations using modular approach with store_date_time_values_as_text: true #3066

Closed HypnoMama closed 4 months ago

HypnoMama commented 4 months ago

Hi there,

I have set up my build.yaml with the same set up as the modular instructions, including the store_date_time_values_as_test: true.

targets:
  $default:
    builders:
      drift_dev:
        # Disable the default builder in favor of the modular builders configured
        # below.
        enabled: false

      drift_dev:analyzer:
        enabled: true
        options: &options
          store_date_time_values_as_text: true
          named_parameters: true
          sql:
            dialect: sqlite
            options:
              version: "3.45.3"
              modules: [fts5]
      drift_dev:modular:
        enabled: true
        options: *options

I have been successfully migrating my db as I go and am now on migration step 9. I have been stuck on running migration tests as I have been setting up my tables with a mixin that adds the id, along with updatedAt and createdAt fields:

mixin BaseEntityMixin on Table {
  IntColumn get id => integer().autoIncrement()();
  DateTimeColumn get createdAt => dateTime().withDefault(currentDateAndTime)();
  DateTimeColumn get updatedAt => dateTime().withDefault(currentDateAndTime)();
}

When I attempt to run a migration test as per the documentation migration tests, I fail the schema validation every time with the same errors:

Schema does not match
table name
  columns: 
    createdAt:
      Different types: Expected INTEGER, got TEXT
    updatedAt:
      Different types: Expected INTEGER, got TEXT

and the same errors, of course, for any another DateTime fields.

I am running the schema validation in development mode, and this passes just fine.

I understand that the error is because we are storing them as text, but the schema is expecting the default integer value and doesn't recognize the change set in the build.yaml. I do not know how to tell the tests or the schema generator that we are storing dateTime values as text, or if there is some other way to allow us to test the migrations.

The createdAt field in the main schema.json file:

{"name":"created_at","getter_name":"createdAt","moor_type":"dateTime","nullable":false,"customConstraints":null,"default_dart":"currentDateAndTime","default_client_dart":null,"dsl_features":[]}

and the generated test schema:

late final GeneratedColumn<DateTime> createdAt = GeneratedColumn<DateTime>(
      'created_at', aliasedName, false,
      type: DriftSqlType.dateTime,
      requiredDuringInsert: false,
      defaultValue: currentDateAndTime);

Thank you for any help you may be able to offer!

simolus3 commented 4 months ago

Thanks for the report! The schema.json file should have a top-level options key containing whether store_date_time_values_as_text was enabled for the particular schema. Is that not included in your schema?

simolus3 commented 4 months ago

Oh wait I think I know what the problem is here... Does this work?

targets:
  $default:
    builders:
      drift_dev:
        # Disable the default builder in favor of the modular builders configured
        # below.
        enabled: false
        options:
          store_date_time_values_as_text: true

      drift_dev:analyzer:
        enabled: true
        options: &options
          store_date_time_values_as_text: true
          named_parameters: true
          sql:
            dialect: sqlite
            options:
              version: "3.45.3"
              modules: [fts5]
      drift_dev:modular:
        enabled: true
        options: *options

We're parsing the build.yaml to check the options from the CLI, that logic doesn't properly account for the other builders.

HypnoMama commented 4 months ago

Hi @simolus3,

Ahhhh yes, thank you so much, that is the problem, indeed. The schema was building with the key set to false. Adding it in properly to the build.yaml causes it to be set to true. Oh man. Words cannot express my gratitude! Thank you!

Sarah

simolus3 commented 4 months ago

No problem, and thanks for the report! The underlying problem has been fixed in f129675139571276694d0fb2a8d98f700904b01c, so the workaround will stop being necessary with the next drift release.