From d91266a460d67726f1c2ad2021c61305477cd62c Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 29 Jul 2026 15:50:33 +0200 Subject: [PATCH 1/2] test: stop config-mutating project tests from sharing a project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five test functions in this package run in parallel and all wrote project configuration to the same two shared fixtures, defaultProject and extraProject. Two pairs conflict directly: - TestPatchProject replaces /services/identity/config/selfservice/ flows/error with an object, while TestPatchKratosConfig sets /selfservice/flows/error/ui_url — the same key underneath, with a different value. - TestPatchPermissionConfig replaces /namespaces with legacy definitions, while TestUpdateNamespaceConfig sets it to an Ory Permission Language location. Whichever landed last won, so the assertions failed intermittently in CI and looked like staging flakiness. Each config-mutating test now gets its own project, leaving defaultProject and extraProject as read-only fixtures for the tests that only read. Subtests within one function still share that project, which is safe as long as each performs a single operation and asserts on the response to it — the invariant patch_permission_config_test.go already documented. The project is created in a workspace of its own because the development-project quota is per workspace; adding to the shared workspace fails with "the quota for the feature 'Development Projects' has been exceeded". TestUpdateProject and TestListProject already create a workspace for the same reason. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JYzGVwAKQ4ormxRHZg1eDu --- cmd/cloudx/project/helper_test.go | 27 +++++++++++++++++++ .../project/patch_identity_config_test.go | 6 +++-- .../project/patch_oauth2_config_test.go | 6 +++-- .../project/patch_permission_config_test.go | 6 +++-- cmd/cloudx/project/patch_test.go | 6 +++-- .../project/update_namespace_config_test.go | 5 ++-- 6 files changed, 46 insertions(+), 10 deletions(-) diff --git a/cmd/cloudx/project/helper_test.go b/cmd/cloudx/project/helper_test.go index aa6257cc..35c4a50b 100644 --- a/cmd/cloudx/project/helper_test.go +++ b/cmd/cloudx/project/helper_test.go @@ -16,6 +16,33 @@ import ( type execFunc = func(stdin io.Reader, args ...string) (string, string, error) +// newProject creates a project for the exclusive use of the calling test. +// +// Tests that write project configuration must not share a project. They run in +// parallel, and several of them write the same keys with different values: for +// example TestPatchProject replaces +// /services/identity/config/selfservice/flows/error with an object, while +// TestPatchKratosConfig sets /selfservice/flows/error/ui_url — the same key +// underneath. Sharing a project makes them clobber each other and fail +// intermittently in CI. +// +// defaultProject and extraProject are therefore read-only fixtures: only tests +// that do not modify project configuration may use them. +// +// Subtests of one test function do share the project this returns, so each must +// still perform a single operation and assert on the response to that operation +// rather than re-reading the project. +// +// The project goes into a workspace of its own because the development-project +// quota is per workspace: adding these to defaultWorkspaceID fails with +// "the quota for the feature 'Development Projects' has been exceeded". This is +// the same reason TestUpdateProject and TestListProject create a workspace +// before their projects. +func newProject(t *testing.T) string { + t.Helper() + return testhelpers.CreateProject(ctx, t, testhelpers.CreateWorkspace(ctx, t)).Id +} + func runWithProjectAsDefault(ctx context.Context, t *testing.T, projectID string, test func(t *testing.T, exec execFunc)) { t.Run("project passed as default", func(t *testing.T) { ctx, _ := testhelpers.WithDuplicatedConfigFile(ctx, t, defaultConfig) diff --git a/cmd/cloudx/project/patch_identity_config_test.go b/cmd/cloudx/project/patch_identity_config_test.go index 47cc3896..baf375fd 100644 --- a/cmd/cloudx/project/patch_identity_config_test.go +++ b/cmd/cloudx/project/patch_identity_config_test.go @@ -14,6 +14,8 @@ import ( func TestPatchKratosConfig(t *testing.T) { t.Parallel() + project := newProject(t) + for _, tc := range []struct { name string doPatch func(t *testing.T, exec execFunc) @@ -53,8 +55,8 @@ func TestPatchKratosConfig(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - runWithProjectAsDefault(ctx, t, defaultProject.Id, tc.doPatch) - runWithProjectAsFlag(ctx, t, extraProject.Id, tc.doPatch) + runWithProjectAsDefault(ctx, t, project, tc.doPatch) + runWithProjectAsFlag(ctx, t, project, tc.doPatch) }) } } diff --git a/cmd/cloudx/project/patch_oauth2_config_test.go b/cmd/cloudx/project/patch_oauth2_config_test.go index a859735a..b2d16723 100644 --- a/cmd/cloudx/project/patch_oauth2_config_test.go +++ b/cmd/cloudx/project/patch_oauth2_config_test.go @@ -14,6 +14,8 @@ import ( func TestPatchHydraConfig(t *testing.T) { t.Parallel() + project := newProject(t) + for _, tc := range []struct { name string doPatch func(t *testing.T, exec execFunc) @@ -51,8 +53,8 @@ func TestPatchHydraConfig(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - runWithProjectAsDefault(ctx, t, defaultProject.Id, tc.doPatch) - runWithProjectAsFlag(ctx, t, extraProject.Id, tc.doPatch) + runWithProjectAsDefault(ctx, t, project, tc.doPatch) + runWithProjectAsFlag(ctx, t, project, tc.doPatch) }) } } diff --git a/cmd/cloudx/project/patch_permission_config_test.go b/cmd/cloudx/project/patch_permission_config_test.go index 364b8fd4..c5519bc9 100644 --- a/cmd/cloudx/project/patch_permission_config_test.go +++ b/cmd/cloudx/project/patch_permission_config_test.go @@ -14,6 +14,8 @@ import ( func TestPatchPermissionConfig(t *testing.T) { t.Parallel() + project := newProject(t) + for _, tc := range []struct { name string // doPatch will use the same project in parallel, so it is important to only do one operation per test @@ -54,8 +56,8 @@ func TestPatchPermissionConfig(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - runWithProjectAsDefault(ctx, t, defaultProject.Id, tc.doPatch) - runWithProjectAsFlag(ctx, t, extraProject.Id, tc.doPatch) + runWithProjectAsDefault(ctx, t, project, tc.doPatch) + runWithProjectAsFlag(ctx, t, project, tc.doPatch) }) } } diff --git a/cmd/cloudx/project/patch_test.go b/cmd/cloudx/project/patch_test.go index b9ffda0c..923ce9bd 100644 --- a/cmd/cloudx/project/patch_test.go +++ b/cmd/cloudx/project/patch_test.go @@ -14,6 +14,8 @@ import ( func TestPatchProject(t *testing.T) { t.Parallel() + project := newProject(t) + for _, tc := range []struct { name string doPatch func(t *testing.T, exec execFunc) @@ -92,8 +94,8 @@ func TestPatchProject(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - runWithProjectAsDefault(ctx, t, defaultProject.Id, tc.doPatch) - runWithProjectAsArgument(ctx, t, extraProject.Id, tc.doPatch) + runWithProjectAsDefault(ctx, t, project, tc.doPatch) + runWithProjectAsArgument(ctx, t, project, tc.doPatch) }) } } diff --git a/cmd/cloudx/project/update_namespace_config_test.go b/cmd/cloudx/project/update_namespace_config_test.go index f0fb4bab..89ef6d17 100644 --- a/cmd/cloudx/project/update_namespace_config_test.go +++ b/cmd/cloudx/project/update_namespace_config_test.go @@ -41,6 +41,7 @@ func TestUpdateNamespaceConfig(t *testing.T) { content := `class Default implements Namespace {}` config := writeFile(t, content) + project := newProject(t) verbs := []string{"update", "patch"} for _, verb := range verbs { @@ -57,8 +58,8 @@ func TestUpdateNamespaceConfig(t *testing.T) { assert.Equal(t, content, data.String(), "the downloaded file does not match what we uploaded") } - runWithProjectAsDefault(ctx, t, defaultProject.Id, updateNamespace) - runWithProjectAsFlag(ctx, t, extraProject.Id, updateNamespace) + runWithProjectAsDefault(ctx, t, project, updateNamespace) + runWithProjectAsFlag(ctx, t, project, updateNamespace) }) } } From a210f3b89ce6abdc613337230fb2aeca8fa273bd Mon Sep 17 00:00:00 2001 From: Arne Luenser Date: Wed, 29 Jul 2026 16:12:01 +0200 Subject: [PATCH 2/2] test: document the isolation invariant and reuse newProject Warn at the declaration site that defaultProject and extraProject are read-only: the rule was only stated inside newProject's doc comment, which someone copying an existing runWithProjectAsDefault call has no reason to open. Explain why both project-selection styles share one project. Each case therefore applies its patch twice in a row, which is safe because the API materializes schema defaults back into the stored config, and is preferred over a project per style: the development-project quota is two per workspace, so that would double both the projects and the workspaces provisioned per run, and the burst makes this package's existing rate-limit flakiness worse. TestUpdateProject provisioned its project by inlining exactly what newProject does, so it now calls the helper. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JYzGVwAKQ4ormxRHZg1eDu --- cmd/cloudx/project/helper_test.go | 17 +++++++++++++---- cmd/cloudx/project/main_test.go | 6 +++++- cmd/cloudx/project/update_test.go | 8 +++----- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/cmd/cloudx/project/helper_test.go b/cmd/cloudx/project/helper_test.go index 35c4a50b..ad4b8407 100644 --- a/cmd/cloudx/project/helper_test.go +++ b/cmd/cloudx/project/helper_test.go @@ -33,11 +33,20 @@ type execFunc = func(stdin io.Reader, args ...string) (string, string, error) // still perform a single operation and assert on the response to that operation // rather than re-reading the project. // +// Callers that exercise both project-selection styles pass this one project to +// both, so each case applies its patch twice in a row and the second +// application runs against already-mutated state. That is safe for the patches +// these tests issue: the API materializes schema defaults back into the stored +// config, so even a repeated `remove` of the same path succeeds. It is also +// preferred over a project per style, which would double both the projects and +// the workspaces provisioned per run — and that burst makes the package's +// existing rate-limit flakiness measurably worse. +// // The project goes into a workspace of its own because the development-project -// quota is per workspace: adding these to defaultWorkspaceID fails with -// "the quota for the feature 'Development Projects' has been exceeded". This is -// the same reason TestUpdateProject and TestListProject create a workspace -// before their projects. +// quota is per workspace, and it is two: adding a third to defaultWorkspaceID +// fails with "the quota for the feature 'Development Projects' has been +// exceeded". This is the same reason TestListProject creates a workspace before +// its projects. func newProject(t *testing.T) string { t.Helper() return testhelpers.CreateProject(ctx, t, testhelpers.CreateWorkspace(ctx, t)).Id diff --git a/cmd/cloudx/project/main_test.go b/cmd/cloudx/project/main_test.go index e6cdee7c..05165e7a 100644 --- a/cmd/cloudx/project/main_test.go +++ b/cmd/cloudx/project/main_test.go @@ -15,7 +15,11 @@ import ( ) var ( - ctx context.Context + ctx context.Context + // defaultProject and extraProject are read-only fixtures shared by every + // test in this package. Tests that write project configuration must not use + // them, because they run in parallel and would clobber each other; call + // newProject instead. defaultProject, extraProject *cloud.Project defaultConfig, defaultWorkspaceID string defaultCmd *cmdx.CommandExecuter diff --git a/cmd/cloudx/project/update_test.go b/cmd/cloudx/project/update_test.go index e7cc2b1b..a26c768c 100644 --- a/cmd/cloudx/project/update_test.go +++ b/cmd/cloudx/project/update_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/ory/cli/cmd/cloudx/testhelpers" "github.com/ory/x/assertx" "github.com/ory/x/cmdx" "github.com/ory/x/snapshotx" @@ -31,8 +30,7 @@ var ( func TestUpdateProject(t *testing.T) { t.Parallel() - workspace := testhelpers.CreateWorkspace(ctx, t) - project := testhelpers.CreateProject(ctx, t, workspace) + project := newProject(t) for _, tc := range []struct { subcommand, @@ -81,7 +79,7 @@ func TestUpdateProject(t *testing.T) { t.Skip("TODO") t.Parallel() - stdout, _, err := defaultCmd.Exec(nil, "update", tc.subcommand, project.Id, "--format", "json", "--file", tc.pathSuccess) + stdout, _, err := defaultCmd.Exec(nil, "update", tc.subcommand, project, "--format", "json", "--file", tc.pathSuccess) require.NoError(t, err) assertx.EqualAsJSONExcept(t, tc.fixture, json.RawMessage(stdout), []string{ @@ -156,7 +154,7 @@ func TestUpdateProject(t *testing.T) { if tc.projectFlag != "" { args = append(args, tc.projectFlag) } - args = append(args, project.Id) + args = append(args, project) stdout, stderr, err := defaultCmd.Exec(nil, args...) require.ErrorIs(t, err, cmdx.ErrNoPrintButFail)