CCOR-13193 - adding test run against oss server in ci - #497
Merged
chrishagglund-ship-it merged 14 commits intoSep 21, 2026
Merged
Conversation
chrishagglund-ship-it
marked this pull request as ready for review
August 19, 2026 17:56
dfont-orkes
reviewed
Sep 3, 2026
dfont-orkes
reviewed
Sep 17, 2026
dfont-orkes
approved these changes
Sep 18, 2026
chrishagglund-ship-it
force-pushed
the
e2e-against-conductor-with-local-script
branch
2 times, most recently
from
September 21, 2026 15:25
0e08ab5 to
99ad862
Compare
…tched instead of re-fetching it, halving poll requests and fixing its always-empty timeout log.
…or fork repo PRs which won't get access to the variable.
Ports javascript-sdk#176's resolution of the same review feedback, so the OSS harness stays diffable across the SDKs. csharp-sdk#171, java-sdk#156 and go-sdk#279 carry the identical change. One tag, one home. The tag was written twice: FORK_PR_FALLBACK_VERSION in the integration-tests-oss job, and `latest` as the local script's default. A local run therefore could not reproduce a CI failure, which is most of why the script exists. Rather than teach one of them to read the other, make the `image:` line of docker-compose-oss.yaml the only place it is written and let both fall through to it: the script applies no default of its own and exports OSS_CONDUCTOR_VERSION only when --version actually supplied one, and the fork-PR branch of the resolve step now leaves the variable unset instead of pinning its own copy. Fork CI and a plain local run reach the identical image by the identical path, with no YAML parsing on either side and one hardcode removed rather than a mechanism added. The non-fork empty case still fails loudly. Worth stating plainly: E2E_TEST_OSS_CONDUCTOR_VERSION is currently set to `latest` at the org level, so the pin is nominal for normal CI runs until someone sets it to a real version. The compose default is what actually holds the line today, on fork PRs and locally. The script's "Using ..." and "Pulling ..." lines now come from `compose config --images` instead of reconstructing the tag, so they stay honest whichever source supplied it. Note the grep: `--images` lists every service's image and does not reliably honour the service-name filter it accepts, so the server's image is selected by name rather than by position. This stack has three services and postgres:16 sorts first, so position would have been wrong here. Pull in CI. The script pulls before `up`, CI did not. On a GitHub-hosted runner the VM is ephemeral and starts with no cached copy, so `up` pulls anyway and the step is redundant today — kept regardless, because it costs no extra network pull (`up` then finds the image locally), it splits "couldn't pull the image" from "the stack didn't come up" into two distinct red steps, and it is what stops a mutable tag going stale the day this job moves to a self-hosted runner with a warm Docker daemon. It also prints the tag in use, which matters now that a fork PR's tag is not spelled out in the workflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The marker description said "excluded from CI", which stopped being true once integration-tests-oss existed: that job runs --bucket=all, which applies no -m filter, so the three marked cases (test_02 sync, test_02 async, test_03_no_performance_overhead) do run there. Reading the description alone, deselecting them in the OSS job looks like the consistent thing to do -- it isn't. The carve-out is server-specific, not test-specific. It exists because the shared sdkdev server doesn't reliably fire a server-side task timeout on a CI-bounded timeline, so the long-sync/long-async buckets deselect those cases. A dedicated local OSS stack doesn't have that problem: all seven --bucket=all OSS runs on this branch are green, and the per-test logs on five of them show all three marked cases passing every time. It is also their only CI coverage -- core ignores both lease files and the long-* buckets deselect the marker -- so filtering them out of the OSS job would leave the server-side timeout path unexercised. Restores the rationale comment on integration-tests-oss, which was in the commit that introduced --bucket=all (458cd6b) and was trimmed by a later cleanup pass (7bf2eec), reworded so "the carve-out does not apply" can't be misread as "the test is skipped". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
chrishagglund-ship-it
force-pushed
the
e2e-against-conductor-with-local-script
branch
from
September 21, 2026 16:57
99ad862 to
b23a246
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a second integration-test CI job that runs the suite against plain Conductor
OSS, alongside the existing authenticated run against the shared Orkes dev server.
scripts/docker-compose-oss.yamlbrings up Conductor OSS + Postgres + httpbin, andits
image:line is the single place the default tag is written;scripts/run-integration-oss.shruns the same stack locally.Orkes-Enterprise-only surface (Authorization, Secrets, Schema, Service Registry,
metadata/scheduler tags) is gated behind
CONDUCTOR_SERVER_TYPE=ossand skips onOSS. The Signal API tests do run on OSS, using WAIT-based fixture variants
(
complex_wf_signal_test_ossand friends) — YIELD is Orkes-only, so the existingfixtures never reach a signalable state there. A few call sites need an OSS-specific
path rather than a skip (archiving on delete, the missing
/workflow/{id}/status,create-with-
overwrite); each is gated on OSS on purpose, so the Orkes path stillfails if the real behaviour regresses.
Requires the
E2E_TEST_OSS_CONDUCTOR_VERSIONorg variable, with a repositoryaccess policy that includes this repo. The new job fails fast if it can't resolve
an image tag — except on a fork PR, where GitHub withholds the variable and the job
falls through to the compose file's default rather than failing.
Also fixes three latent bugs in the existing Orkes run
Running against a fresh, single-tenant server surfaced these. All three affect the
integration-testjob too, not just the new one:test_async.pywas clobbering another suite's task def — it registered a bareTaskDefunderpython_integration_test_task, the nametest_workflow_execution.pyruns real workflows against. Those two run asconcurrent CI jobs against one server, so last writer won, and a bare def gets the
server's default 3600s
responseTimeoutSeconds— so a stalled worker held the taskfor an hour and the workflow sat
IN_PROGRESS. It now registers its own def instead.run_workflow_execution_testsnever registered its task def —generate_tasks_defs()was dead code, so the def was whatever another suite leftbehind. Registering it sets
response_timeout_seconds=2(see 1) and makes the suitework on a fresh server.
_wait_for_blocking_taskmatched any in-flight task, but the outerSUB_WORKFLOWflips to
IN_PROGRESSbefore the nested workflow reaches its blocking task, so thesignal raced the real block (intermittent "Expected BLOCKING_TASK, got None" / 404
"Found no blocked task"). It now descends into running sub-workflows and matches
only a non-terminal WAIT/YIELD task.
Also
is_oss()helper instead of 9 open-codedenv reads, 4 of them inverted.
test_task_metadata_service.pygains atearDownClass— its defs used to be leftbehind, so re-runs hit "already exists".
test@conductoross.ioplaceholder csharp-sdk settledon for this ticket, not real employee addresses.
run-integration-oss.shand the CI job both pull the image unconditionally so acached mutable tag isn't reused; OSS health wait 120s → 180s;
workflow_dispatchgains an
oss_conductor_versioninput.