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

[dullbananas]

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

[momentary-lapse]

I can take this one

[momentary-lapse]

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

[Nutomic]

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

[momentary-lapse]

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?

[Nutomic]

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

[momentary-lapse]

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

[dessalines]

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

[Nutomic]

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

[momentary-lapse]

• 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

[dessalines]

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.

[momentary-lapse]

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

[Nutomic]

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