Skip to content

Run the integration suite in CI - #489

Open
pksgit wants to merge 5 commits into
mainfrom
pradeepsharma/cs-2036-run-the-ingress-integration-tests-in-ci
Open

Run the integration suite in CI#489
pksgit wants to merge 5 commits into
mainfrom
pradeepsharma/cs-2036-run-the-ingress-integration-tests-in-ci

Conversation

@pksgit

@pksgit pksgit commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

CS-2036. Nothing behind the integration build tag has ever run on a pull
request, and test/ is not even compiled, so a build error there goes green
today. Found while adding an integration test in #480.

Two independent gaps

  1. The integration job in build.yaml has a single step, docker build -f ./build/ingress/Dockerfile. It never invokes go test -tags integration.
  2. build/test/Dockerfile copies only pkg/ and version/, so test/ is neither compiled nor linted. .golangci.yaml already sets build-tags: [integration], so the config was right all along; the files were never in the image for it to read.

What this does

build/test/Dockerfile also copies cmd/ and test/, builds the ingress
binary the suite spawns as its handler, and compiles the suite ahead of time so a
break under test/ fails the image. The existing Test job shares that image
and gains the same coverage for about 20 seconds.

build/test/entrypoint.sh gains an INTEGRATION branch that runs the prebuilt
suite. Without it the unit path runs exactly as before.

test-integration.yaml runs the suite against a Redis and a livekit-server
in dev mode, both containers on the host network. Dev mode supplies the
devkey/secret pair the generated config uses, so this needs no repository
secrets and works on a fork. That is a deliberate departure from the egress
workflow, which takes its config from secrets.EGRESS_CONFIG_STRING pointing at
a remote room server.

Redis carries this suite and nothing else, on purpose: any other service
registering an IOInfoServer on the same bus takes a share of the state updates
and the ingress under test never reaches a terminal state. A separate Redis
database does not help, since psrpc rides pub/sub and pub/sub is not scoped to
one. That comment is in the file so nobody folds it into a shared service later.

The matrix covers rtmp and url through the existing rtmp_only and
url_only config fields, so no new Go code. An INTEGRATION_TYPE selector of
the kind egress has belongs with the harness refactor in CS-2037.

Verified locally

Check Result
docker build -f build/test/Dockerfile, lint stage uncached exit 0, 0 issues with test/ in scope
Lint really reads test/ planted // recieve in test/url.go, rebuilt --target lint: failed with misspell, exit 1
go test -c -tags integration ./test/ compiles in the image
ingress on PATH, test.test present both
RTMP publisher's GStreamer elements flvmux, rtmp2sink, audiotestsrc, faac, videotestsrc, x264enc all present
Unit path unchanged (INTEGRATION unset) go test ./pkg/..., 36 tests, 0 failures
rtmp leg with this workflow's config PASS 48.1s, audio and simulcast video published
url leg PASS 48.2s, 2 tracks

Draft, because one thing cannot be checked locally

Whether --network host behaves as expected on ubuntu-latest, and whether a
livekit-server container completes ICE with a handler in a sibling container
sharing that network. Both legs above ran natively against a local Redis and
room server, which exercises the config, the credentials and the publishers, but
not the container topology. This PR's own run settles it, since the paths filter
matches the files it changes.

Worth knowing before review

Green here means "did not error", not "media verified". Both tests end on
require.NotEqual(t, final.State.Status, ENDPOINT_ERROR) and each run produced a
single state update. Media did flow in both local runs, but nothing asserts it.
Pre-existing, and the clearest argument for the checkUpdate helper in CS-2037.

The url leg reaches devimages.apple.com, which RunURLTest has pulled since
Support for pulling URLs (#138). It is the only external URL in the Go test
code, and lighter than what egress already depends on.

WHIP is left out: its publisher is the livekit-whip-bot submodule, which has to
be checked out and built into the image first. Stated in the workflow rather than
left implicit.

redis:7-alpine and livekit/livekit-server:v1.13.6 are version-pinned rather
than digest-pinned, unlike the action SHAs and GSTVERSION.

🤖 Generated with Claude Code

Nothing behind the integration build tag has ever run on a pull request.
The integration job builds the ingress image and stops there, and the test
image copies only pkg/ and version/, so test/ is neither compiled nor
linted and a build error there goes green.

Copy cmd/ and test/ into the test image, build the ingress binary the
suite spawns as its handler, and compile the suite ahead of time so a
break under test/ fails the image. The unit test job shares that image and
so gains the same coverage.

Add a workflow that runs the suite against a Redis and a livekit-server in
dev mode, both as containers on the host network. Dev mode supplies the
devkey/secret pair the generated config uses, so this needs no repository
secrets and runs on a fork. Redis carries nothing else: another service
registering an IOInfoServer on the same bus takes a share of the state
updates and the ingress under test never reaches a terminal state.

The matrix covers rtmp and url through the existing rtmp_only and url_only
config flags. WHIP needs the livekit-whip-bot submodule built into the
image first.

CS-2036

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pksgit
pksgit marked this pull request as ready for review September 8, 2026 17:41
@pksgit
pksgit requested a review from a team as a code owner September 8, 2026 17:41

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

Comment thread .github/workflows/test-integration.yaml Outdated
Comment thread .github/workflows/test-integration.yaml Outdated
@milos-lk

milos-lk commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

what do you think about have a docker compose file like this:

# build/test/compose.yaml
services:
  redis:
    image: redis:7-alpine
    healthcheck: { test: ["CMD", "redis-cli", "ping"], interval: 1s, retries: 30 }
  livekit:
    image: livekit/livekit-server:v1.13.6
    command: --dev
    healthcheck: { test: ["CMD", "wget", "-qO-", "http://localhost:7880"], interval: 1s, retries: 30 }
  test:
    build: { context: ../.., dockerfile: build/test/Dockerfile }
    depends_on:
      redis: { condition: service_healthy }
      livekit: { condition: service_healthy }
    environment:
      INTEGRATION: "1"
      INGRESS_CONFIG_BODY: ${INGRESS_CONFIG_BODY}

The generated config then points at service names and it would allow us to easily share it between the CI workflow docker compose -f build/test/compose.yaml run --rm test and in magefile we could reuse that (by adding a target like: IntegrationDocker(configFile))

pksgit and others added 3 commits September 10, 2026 09:16
The workflow started Redis and a room server with docker run, polled each
with its own retry loop, and built the image in a separate step. A compose
file carries all of that, and the same file runs on a developer's machine
through mage IntegrationDocker, so a local pass and a green check mean the
same thing. Integration still runs the suite natively against Homebrew
GStreamer, which is a different environment from CI.

The services find each other by name on a compose network rather than
sharing the host's. That is why the room server needs an explicit bind:
dev mode listens on loopback only, so without it every other container is
refused, and a healthcheck on localhost reports the service up while
nothing can reach it.

CS-2036

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
.gst-version arrived on main and the test Dockerfile now takes GSTVERSION
as a build arg with no default, so a build without it fails at parse time
rather than using a stale base image. The compose file and the workflow
pass it, read from the pin the same way test.yaml reads it, and mage takes
it from the getGstVersion helper that came with it.

A GStreamer bump is now a reason to run this suite, so .gst-version joins
the paths filter.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
compose run starts whatever image already exists and only builds when
there is none, so a second run after a source or Dockerfile change tests
the previous build. A local run silently passed against an image two
GStreamer versions behind the pin.

CS-2036

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

.go-version, go.sum and version/ are copied into the test image but were
missing from the paths filter, so a pull request touching only those got
no integration run at all rather than one that passed. Two such changes
landed on main last week: a Go toolchain bump that touched .go-version
alone, and a grpc security bump that touched go.mod and go.sum.

CS-2036

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants