From c74ccbe4fc69e85b475a3a9421d5959205149f23 Mon Sep 17 00:00:00 2001 From: Brandur Date: Sat, 4 Apr 2026 20:59:32 -0500 Subject: [PATCH] Expose `TestSchemaOpts.DisableReuse` for test schema creation The `riverdbtest.TestSchema` function has had an internal `disableReuse` option for quite some time that prevents the test schema from being checked back in at the end of a test case. This is useful in cases where a schema might be modified and not suitable for pick up by a subsequent test. I was just testing some schema-related changes in Pro and found that it would be somewhat useful to be able to use this option from that package. We can also create our own test schemas from scratch over there, but `riverdbtest.TestSchema` has a number of niceties like being able to clean up test schemas even in the event of a panic and where cleanup hooks aren't run. I don't think there's any particularly harmful effect in exposing `DisableReuse` (except that it might encourage more of its use), so it's probably okay. --- riverdbtest/riverdbtest.go | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/riverdbtest/riverdbtest.go b/riverdbtest/riverdbtest.go index 259532ae..eb1e6df1 100644 --- a/riverdbtest/riverdbtest.go +++ b/riverdbtest/riverdbtest.go @@ -51,6 +51,15 @@ var ( // TestSchemaOpts are options for TestSchema. Most of the time these can be left // as nil. type TestSchemaOpts struct { + // DisableReuse specifies that schema will not be checked in for reuse at + // the end of tests. This is desirable in certain like cases like where a + // test case is making modifications to schema. + // + // Not being able to reuse the schema introduces overhead to tests because + // it means more schemas will need to be generated to replace one not + // reused, so don't set this option unless necessary. + DisableReuse bool + // LineTargetVersions specify target versions for migration lines being // migrated. By default all lines are migrated all the way up, but this lets // tests migrate to an only partially applied version. This option is rarely @@ -81,11 +90,6 @@ type TestSchemaOpts struct { // instead of `schema.river_job`). ProcurePool func(ctx context.Context, schema string) (any, string) - // disableReuse specifies that schema will not be checked in for reuse at - // the end of tests. This is desirable in certain like cases like where a - // test case is making modifications to schema. - disableReuse bool - // skipPackageNameCheck skips the check that package name doesn't resolve to // `riverdbtest`. Normally we want this to make sure that we're skipping // the right number of frames back to the caller package, but it needs to be @@ -279,7 +283,7 @@ func TestSchema[TTx any](ctx context.Context, tb testutil.TestingTB, driver rive } if withCleanup, ok := tb.(testingTBWithCleanup); ok { - if !opts.disableReuse { + if !opts.DisableReuse { withCleanup.Cleanup(func() { idleSchemasMu.Lock() defer idleSchemasMu.Unlock() @@ -638,15 +642,15 @@ func testTxSchemaForDatabaseAndMigrationLines[TTx any](ctx context.Context, tb t } schema = TestSchema(ctx, tb, driver, &TestSchemaOpts{ - Lines: lines, - ProcurePool: opts.ProcurePool, - // If test transactions are being shared (opts.DisableSharing = false) // then reserve the shared schemas exclusively for TestTx. Otherwise, // allow them to be put back in the pool for use by other test // transactions with opts.DisableSharing = true or other TestSchema // invocations. - disableReuse: !opts.DisableSchemaSharing, + DisableReuse: !opts.DisableSchemaSharing, + + Lines: lines, + ProcurePool: opts.ProcurePool, skipExtraFrames: skipExtraFrames, skipPackageNameCheck: opts.skipPackageNameCheck,