conductor-oss / conductor-oss/conductor-cli

E2E fixtures use fixed global names, so two runs against one server corrupt each other

Open
#119 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
11
Forks
4
PR merge metrics
No merged PRs in 30d

Description

Every E2E suite that creates server-side state uses a hardcoded name, and `setup_file` deletes those names before running to clear leftovers from previous runs. So two runs against the same server delete each other's fixtures mid-test.

This bites the Enterprise job, which targets one shared remote server and runs on every PR. Nothing serialised it, so any two PRs with overlapping CI were corrupting each other — presenting as unexplained E2E failures on unrelated PRs. #114 added a `concurrency` group as a stopgap, so overlapping runs queue rather than interleave. That has its own cost: a queued run can be superseded and report cancelled.

The OSS matrix is unaffected, since each leg downloads and runs its own private server.

## Fix

Give every created fixture a run-scoped suffix from a single variable, defaulted so local runs need no setup.

Most of this is small — six of the seven affected suites already keep their names in variables:

| suite | work |
|---|---|
| `workflow`, `task`, `secret`, `search`, `rerun` | suffix the vars, plus a couple of stray literals each |
| `api_gateway` | same, plus the hardcoded `--path '/api/e2e'` |
| `schedule` | real refactor — no variables, 55 inline literals |

Three parts aren't just renaming:

- `cli_e2e_test_workflow`, `cli_e2e_test_workflow_2` and `cli_e2e_test_task` live in on-disk JSON under `test/e2e/`, so they need templating per run — write a copy to a scratch dir rather than mutating the tracked fixtures, or a failed run leaves the repo dirty.
- `api_gateway` collides on the gateway path as well as the service name. Worth auditing for other non-name uniqueness.
- Namespaced fixtures stop being cleaned by the next run's blanket delete, so `teardown_file` needs to remove its own namespace and something needs to sweep orphans — otherwise the shared server accumulates junk indefinitely.

For `schedule.bats`, extract the variables in one behaviour-preserving commit and apply the suffix in another. A semantic change buried in a 55-site rename is hard to review, and the failure mode is a test passing because it no longer asserts on anything it created.

## Verifying it

Start one local OSS server and run two full suites against it simultaneously; both should pass. Both fail today, so that is the regression test. Also kill a run mid-flight and check the orphan sweep reclaims its fixtures.

## Done when

- Two concurrent runs against one server both pass
- No suite deletes a resource it did not create
- An aborted run leaves nothing that breaks a later run or grows without bound
- Local runs still work with no environment variable set
- The `concurrency` block on `e2e-enterprise` is removed, since it exists only to work around this

Contributor guide

No contributing guide indexed for this repository

Research direction

Read the affected E2E suites under test/e2e/, especially schedule.bats, and the e2e-enterprise workflow. First run two full suites against one local OSS server concurrently to reproduce the corruption. Done means concurrent runs pass, aborted runs leave no lasting fixtures, local runs need no setup, and the workflow concurrency workaround can be removed.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, shell
Domain
ci-cd, testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.