lemmy icon indicating copy to clipboard operation
lemmy copied to clipboard

In migration test, do inserts before some migrations to avoid empty tables

Open dullbananas opened this issue 9 months ago • 12 comments

This would catch errors that only happen if some tables have at least one row, such as #5531

dullbananas avatar Mar 25 '25 00:03 dullbananas

I can take this one

momentary-lapse avatar May 12 '25 16:05 momentary-lapse

Are you considering any specific (common for diesel) approach to implement these test migrations, or this ticket assumes figuring it out?

momentary-lapse avatar May 21 '25 20:05 momentary-lapse

The mentioned test is here. Implementing this would work more or less so:

  • Set Options.limit to only run the first few migrations (enough to create tables for user, community, post, comment)
  • Use raw sql to insert some rows (as diesel structs are not compatible with the old db schema)
  • Run all the remaining migrations
  • Read the inserted data with normal diesel functions (eg Post::read)
  • Check that fields like post.title are unchanged
  • Keep the remaining logic to check revert migration etc

Nutomic avatar May 22 '25 07:05 Nutomic

Do you think it's worth to invest into a more extensible solution? Like, a module with test migrations that run after corresponding migration is finished?

momentary-lapse avatar May 22 '25 08:05 momentary-lapse

Not sure what you mean, but its better to keep things simple and avoid unnecessary complexity.

Nutomic avatar May 22 '25 08:05 Nutomic

Okay, I see that for now it's enough to add test data once. I've been thinking about situation when it's not enough, and you need to run, for instance, 10 migrations then add some test data, then next 15 migrations and add more data, then 20 and more data etc. But it's not the case

momentary-lapse avatar May 22 '25 09:05 momentary-lapse

I'm also not sure this would work, because it'd need custom tests after specific migrations have been run.

dessalines avatar May 23 '25 00:05 dessalines

Its enough to check after all migrations are finished that the test data still exists.

Nutomic avatar May 23 '25 08:05 Nutomic

  • Read the inserted data with normal diesel functions (eg Post::read)

Looks like this is impossible, since db_schema module containing Post, Person, Community, etc. models depends on db_schema_file where the test is located. I'm gonna use plain diesel then. Or, maybe you've considered to reorganize crates? db_schema_file doesn't seem very big, it might be okay to merge them

momentary-lapse avatar Jun 08 '25 20:06 momentary-lapse

IMO there are better test-related issues to take than this one, like #4979 .

I'm also still not clear on which specific migrations are run before we test them for empty data, 10 migrations, 20? Or how many initial migrations are done. That makes this type of test custom and difficult to design.

dessalines avatar Jun 09 '25 21:06 dessalines

By the way, i already experimented with that number and found that around 40th migration there are some mandatory apub fields are introduced (public_key, actor_id, ap_id). In one of the next migrations, all the entries not having these fields are being deleted. So i guess it should be at least 40 migrations, maybe even more—say, take a state from 2-3 years ago.

But yeah, i may switch to the more important one

momentary-lapse avatar Jun 09 '25 21:06 momentary-lapse

Right then you also have to use raw sql to validate data after migrations.

Nutomic avatar Jun 10 '25 10:06 Nutomic