diff --git a/cmd/cloudx/project/helper_test.go b/cmd/cloudx/project/helper_test.go index aa6257cc..ad4b8407 100644 --- a/cmd/cloudx/project/helper_test.go +++ b/cmd/cloudx/project/helper_test.go @@ -16,6 +16,42 @@ 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. +// +// 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, 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 +} + 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/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/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) }) } } 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)