Skip to content

Improve migration integrity tests #16577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 24, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { context } from "@budibase/backend-core"
import { context, DesignDocuments } from "@budibase/backend-core"
import * as setup from "../../api/routes/tests/utilities"
import * as migrations from "../migrations"
import { getAppMigrationVersion, updateAppMigrationMetadata } from ".."
import { processMigrations } from "../migrationsProcessor"
import { Database } from "@budibase/types"

describe("migration integrity", () => {
// These test is checking that each migration is "idempotent".
Expand All @@ -9,16 +12,55 @@ describe("migration integrity", () => {
const config = setup.getConfig()
await config.init()

await config.doInContext(config.getAppId(), async () => {
const db = context.getAppDB()
for (const migration of migrations.MIGRATIONS) {
await migration.func()
const docs = await db.allDocs({ include_docs: true })
async function setCurrentVersion(currentMigrationId: string) {
for (const appId of [config.getAppId(), config.getProdAppId()]) {
await config.doInContext(appId, async () => {
await updateAppMigrationMetadata({
appId,
version: currentMigrationId,
})
})
}
}

async function getDocs(db: Database) {
const allDocs = await db.allDocs({ include_docs: true })
const documentsToIgnore: string[] = [DesignDocuments.MIGRATIONS]
return {
...allDocs,
rows: allDocs.rows.filter(r => !documentsToIgnore.includes(r.id)),
}
}

const appId = config.getAppId()
await config.doInContext(appId, async () => {
await setCurrentVersion("")
const devDb = context.getAppDB()
const prodDb = context.getProdAppDB()
for (let i = 0; i < migrations.MIGRATIONS.length; i++) {
const migrationsToApply = migrations.MIGRATIONS.slice(0, i + 1)
const latestMigration =
migrationsToApply[migrationsToApply.length - 1].id

const currentVersion = await getAppMigrationVersion(appId)

await processMigrations(appId, migrationsToApply)
expect(await getAppMigrationVersion(appId)).toBe(latestMigration)

const devDocs = await getDocs(devDb)
const prodDocs = await getDocs(prodDb)

await setCurrentVersion(currentVersion)
expect(await getAppMigrationVersion(appId)).not.toBe(latestMigration)

await processMigrations(appId, migrationsToApply)
expect(await getAppMigrationVersion(appId)).toBe(latestMigration)

await migration.func()
const latestDocs = await db.allDocs({ include_docs: true })
const latestDevDocs = await getDocs(devDb)
const latestProdDocs = await getDocs(prodDb)

expect(docs).toEqual(latestDocs)
expect(latestDevDocs).toEqual(devDocs)
expect(latestProdDocs).toEqual(prodDocs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these valid assertions? Aren't migrations by definition going to change some documents?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what this test is testing. Each migration should be idempotent, and you should be able to run it multiple times. If it succeeded once, subsequent runs should not change anything (as everything is migrated already). I renamed the variables to make it more verbose

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh I totally misunderstood, the rename is much better. Thank you 🙏

}
})
})
Expand Down
92 changes: 92 additions & 0 deletions packages/server/src/sdk/users/tests/utils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { db, roles } from "@budibase/backend-core"
import { structures } from "@budibase/backend-core/tests"
import { sdk as proSdk } from "@budibase/pro"
import tk from "timekeeper"

import TestConfiguration from "../../../tests/utilities/TestConfiguration"
import { rawUserMetadata, syncGlobalUsers } from "../utils"
Expand All @@ -9,6 +10,7 @@ describe("syncGlobalUsers", () => {
const config = new TestConfiguration()

beforeEach(async () => {
tk.reset()
await config.init()
})

Expand Down Expand Up @@ -62,6 +64,96 @@ describe("syncGlobalUsers", () => {
})
})

it("app users are synced", async () => {
const initalDate = new Date()
tk.freeze(initalDate)
const user1 = await config.createUser({
admin: { global: false },
builder: { global: false },
roles: {
[config.getProdAppId()]: roles.BUILTIN_ROLE_IDS.BASIC,
},
})
const user2 = await config.createUser({
admin: { global: false },
builder: { global: false },
})
await config.doInContext(config.appId, async () => {
let metadata = await rawUserMetadata()
expect(metadata).not.toContainEqual(
expect.objectContaining({
_id: db.generateUserMetadataID(user1._id!),
})
)
expect(metadata).not.toContainEqual(
expect.objectContaining({
_id: db.generateUserMetadataID(user2._id!),
})
)

tk.freeze(new Date(Date.now() + 1000))
await syncGlobalUsers()

metadata = await rawUserMetadata()
expect(metadata).toContainEqual({
_id: db.generateUserMetadataID(user1._id!),
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),

email: user1.email,
firstName: user1.firstName,
lastName: user1.lastName,
builder: { global: false },
admin: { global: false },

roleId: "BASIC",
tableId: "ta_users",
tenantId: config.getTenantId(),
_rev: expect.stringMatching(/^1-\w+/),
})
expect(metadata).not.toContainEqual(
expect.objectContaining({
_id: db.generateUserMetadataID(user2._id!),
})
)
})
})

it("app users audit data is updated", async () => {
tk.freeze(new Date())
const user1 = await config.createUser({
admin: { global: false },
builder: { global: false },
roles: {
[config.getProdAppId()]: roles.BUILTIN_ROLE_IDS.BASIC,
},
})
await config.doInContext(config.appId, async () => {
tk.freeze(new Date(Date.now() + 1000))
const updatedTime = new Date()

await syncGlobalUsers()

tk.freeze(new Date(Date.now() + 1000))
await config.createUser({
...user1,
firstName: "updatedName",
})
tk.freeze(new Date(Date.now() + 1000))

await syncGlobalUsers()

const metadata = await rawUserMetadata()
expect(metadata).toContainEqual(
expect.objectContaining({
_id: db.generateUserMetadataID(user1._id!),
createdAt: updatedTime.toISOString(),
updatedAt: new Date().toISOString(),
})
)
})
})

it("app users are not synced if not specified", async () => {
const user = await config.createUser({
admin: { global: false },
Expand Down
4 changes: 4 additions & 0 deletions packages/server/src/sdk/users/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export function combineMetadataAndUser(
// copy rev over for the purposes of equality check
if (found) {
newDoc._rev = found._rev
newDoc.createdAt = found.createdAt
newDoc.updatedAt = found.updatedAt
}
// clear fields that shouldn't be in metadata
delete newDoc.password
Expand All @@ -53,6 +55,8 @@ export function combineMetadataAndUser(
return {
...found,
...newDoc,
createdAt: found?.createdAt ?? (new Date().toISOString() as any),
updatedAt: new Date().toISOString(),
}
}
return null
Expand Down