Migrations test suite
Just a shower thought, but after adding some migration tests in !2792 (merged) I had some ideas about how we could create a test suite around migrations.
Why?
First, why? I think in theory, a migration is supposed to be very straightforward - it either succeeds or fails. However in Pleroma we're doing a lot of complex migrations, so sometimes a passing migration even with passing unit tests still results in a failure. If there's any way we can verify migrations with additional tests, that would be a big help.
I noticed this is a recurring issue in Pleroma. Since I've joined the mailing list, at least one other comes to mind: !1877 (comment 55889) And now I've made the mistake too.
Example test
A ConfigDB migration is the perfect candidate for a migration test, since it doesn't require any schema changes and will continue to work in the future. Below is a migration I added to !2792 (merged) to fix a malformed config, and a test to accompany it.
priv/repo/migrations/20200722185515_fix_malformed_formatter_config.exs:
# This migration will fix the config of Pleroma.Formatter when it's stored
# incorrectly. A previous migration incorrectly stored the config as a map,
# and this will convert it into a list (the correct format).
defmodule Pleroma.Repo.Migrations.FixMalformedFormatterConfig do
use Ecto.Migration
alias Pleroma.ConfigDB
@config_path %{group: :pleroma, key: Pleroma.Formatter}
def change do
# The with-statement only matches a map, so it will be skipped if the
# config is already a list
with %ConfigDB{value: %{} = opts} <- ConfigDB.get_by_params(@config_path),
fixed_opts <- Map.to_list(opts) do # Convert the config to a list
fix_config(fixed_opts)
else
# We return an atom so the test can check what happened
_ -> :skipped
end
end
defp fix_config(fixed_opts) when is_list(fixed_opts) do
{:ok, _} =
# Update ConfigDB with the fixed data
ConfigDB.update_or_create(%{
group: :pleroma,
key: Pleroma.Formatter,
value: fixed_opts
})
# Again, we return :ok so the test can check the result
:ok
end
end
Now for the test:
test/migrations/20200722185515_fix_malformed_formatter_config_test.exs:
defmodule Pleroma.Repo.Migrations.FixMalformedFormatterConfigTest do
use Pleroma.DataCase
import Pleroma.Factory
import Pleroma.Tests.Helpers
alias Pleroma.ConfigDB
setup do: clear_config(Pleroma.Formatter)
# Import the migration module.
# It gets passed to each test as `%{migration: migration}`
setup_all do: require_migration("20200722185515_fix_malformed_formatter_config")
test "change/0 skips if Pleroma.Formatter is empty", %{migration: migration} do
# Here the migration gets run by calling change/0
# Since the database is in its default state, it won't have any malformed config,
# so we expect the migration logic to be skipped.
assert :skipped == migration.change()
end
test "change/0 skips if Pleroma.Formatter config is already a list", %{migration: migration} do
opts = [
class: false,
extra: true,
new_window: false,
rel: "ugc",
strip_prefix: false
]
# Here we test a correctly-formatted ConfigDB value.
# We insert it into ConfigDB.
insert(:config, group: :pleroma, key: Pleroma.Formatter, value: opts)
# Again, the migration gets run by calling change/0
# We expect it to return :skipped since the config isn't malformed
assert :skipped == migration.change()
# Sanity check that the data is in the database
%{value: new_opts} = ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Formatter})
assert new_opts == opts
end
test "change/0 converts a map into a list", %{migration: migration} do
incorrect_opts = %{
class: false,
extra: true,
new_window: false,
rel: "F",
strip_prefix: false
}
# Finally, we insert a malformed config. This is what the migration should fix.
insert(:config, group: :pleroma, key: Pleroma.Formatter, value: incorrect_opts)
# Again, we run the migration.
# It should return :ok since the config was malformed, and it fixed it.
assert :ok == migration.change()
# Sanity check the updated config
%{value: new_opts} = ConfigDB.get_by_params(%{group: :pleroma, key: Pleroma.Formatter})
# Sanity check the updated config
assert new_opts == [
class: false,
extra: true,
new_window: false,
rel: "F",
strip_prefix: false
]
# Put the new config into Pleroma's environment for the next test.
# I couldn't figure out how to do this all in one step.
Pleroma.Config.put(Pleroma.Formatter, new_opts)
assert new_opts == Pleroma.Config.get(Pleroma.Formatter)
# Now we do an actual test on something that uses the config.
# This is just another sanity check to make sure our transformed config
# is in fact valid for its purpose now.
{text, _mentions, []} =
Pleroma.Formatter.linkify(
"https://www.businessinsider.com/walmart-will-close-stores-on-thanksgiving-ending-black-friday-tradition-2020-7\n\nOmg will COVID finally end Black Friday???"
)
assert text ==
"<a href=\"https://www.businessinsider.com/walmart-will-close-stores-on-thanksgiving-ending-black-friday-tradition-2020-7\" rel=\"F\">https://www.businessinsider.com/walmart-will-close-stores-on-thanksgiving-ending-black-friday-tradition-2020-7</a>\n\nOmg will COVID finally end Black Friday???"
end
end
Building on this
Since a lot of people depend on Pleroma, and migrations are one of the few areas without regimented testing, I'd like to try improving the situation.
In general, I think tests like the one above should always be doable for migrations that don't require schema changes. I see no reason not to include tests for them.
For migrations which do make schema changes, I think we'd need a test runner that walks through the migrations one at a time, and runs a test if it exists. This seems pretty doable with a custom mix task. It could do something like this:
mix pleroma.testmigrations
- It creates a fresh
pleroma_migrations_test
db - It iterates through each migration, one at a time
- If a test called
priv/repo/test/"#{current_migration}_test.exs
exists, it runs it before the migration - If the test passed, it keeps iterating. It recreates the database as needed.
- Could be a final pipeline in GitLab CI
To be clear, I don't think all migrations need tests. If it's just a schema migration, it doesn't need a test. Migrations only need tests if they have a bunch of logic that's difficult to verify, and if they have potential side-effects that can't be tested by unit tests alone.
The benefit of walking through each migration is that we could seed the test data at that stage of the database, then verify the result of the migration. We'd have to use raw SQL since we can't rely on Pleroma's API staying the same.
Just wanted to write these ideas down somewhere while they were still fresh in my mind.