dev-qemu: Add integration test to CI#14
Merged
Merged
Conversation
Assisted-by: GitHub Copilot:claude-opus-4.6 Signed-off-by: Kurtis Dinelle <kdinelle@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Adds a basic CI integration-test job that boots the dev-qemu platform in QEMU and exercises ec-test-cli commands against it, providing an early deadlock/regression signal for the service plumbing.
Changes:
- Add
scripts/integration-test.shto startdev-qemu, discover its PTY serial device, and run a suite ofec-test-clirequests. - Add a
run-headlessCargo alias fordev-qemuto run without defmt output and with a CI-friendly runner. - Add an
integration-testjob to.github/workflows/check.ymlthat installs QEMU +ec-test-cliand runs the script.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| scripts/integration-test.sh | New integration test driver script that launches QEMU and runs ec-test-cli commands. |
| platform/dev-qemu/.cargo/config.toml | Adds run-headless Cargo alias to run dev-qemu without defmt logging and with an explicit QEMU runner. |
| .github/workflows/check.yml | Adds a new GitHub Actions job to install dependencies and execute the integration test script. |
Assisted-by: GitHub Copilot:claude-opus-4.6 Signed-off-by: Kurtis Dinelle <kdinelle@microsoft.com>
RobertZ2011
previously approved these changes
May 8, 2026
Assisted-by: GitHub Copilot:claude-opus-4.6 Signed-off-by: Kurtis Dinelle <kdinelle@microsoft.com>
tullom
previously approved these changes
May 12, 2026
jerrysxie
reviewed
May 12, 2026
kurtjd
force-pushed
the
ci-integration-test
branch
from
c36dd11 to
459433e
Compare
kurtjd
force-pushed
the
ci-integration-test
branch
from
459433e to
9c3554b
Compare
jerrysxie
approved these changes
May 12, 2026
tullom
approved these changes
May 13, 2026
github-project-automation
Bot
moved this from In review
to Done
in Engineering System & Infrastructure
This was referenced May 20, 2026
Merged
dymk
added a commit
to OpenDevicePartnership/odp-platform-common
that referenced
this pull request
May 20, 2026
The serial transport in ec-test-lib currently sends MCTP frames without the trailing SMBus PEC (Packet Error Code) byte that the SMBus protocol specifies as the trailing byte of each frame. embedded-services' uart-service historically compensated for this on the device side by stripping the PEC byte on TX and skipping the PEC byte on RX. embedded-services PR OpenDevicePartnership/embedded-services#852 makes uart-service strictly spec-compliant: SmbusEspiMedium now emits a PEC byte on every TX frame and requires a PEC byte on every RX frame. Without a matching change on the host side, `ec-test-cli --source serial` against any post-#852 uart-service consumer (dev-imxrt, dev-mcxa, dev-npcx, dev-qemu) hangs waiting for a PEC byte the EC never sends, and the extra PEC byte sent by ec-test-cli corrupts the EC's next request. This change adds PEC computation on TX (one byte appended after the existing frame) and verification on RX (one byte read + compared against `smbus_pec::pec()` over the received frame), using the `smbus_pec` crate already in the transitive dependency graph. Coordination note: this PR and embedded-services #852 must land in tight sequence. After #852 merges and `odp-embedded-controller` bumps its `embedded-services` pin past it, `EC_TEST_CLI_REV` in `.github/workflows/check.yml` must be bumped past this commit at the same time, otherwise the `integration-test:` CI job added by OpenDevicePartnership/odp-embedded-controller#14 will fail. Validated end-to-end against `odp-embedded-controller`'s `scripts/integration-test.sh` running dev-qemu built against embedded-services with PR #852 applied: all 18 commands (thermal x8, battery x3, rtc x7) succeed. Assisted-by: GitHub Copilot CLI:claude-opus-4.7-1m-internal
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 basic integration testing against
dev-qemuin CI.dev-qemuinstance and makes each service request against it from theec-test-cliapp. Right now, we only check that the request completed successfully which is still useful to ensure things aren't completely broken/deadlocked, but don't have good plumbing in place to check the returned results against a range of expected results (that's next on the todo list).run-headlessalias todev-qemuwhich runs the platform without defmt logging. Mainly useful so we don't need to installdefmt-printduring the CI job (since we don't really need the logs).integration-testwhich kicks off the script on PR.The idea is this will mainly catch any significant breakage in the integration of the services as we update
embedded-serviceshere.Note:
odp-platform-commonis still private so the job will likely fail at the moment until it's made public (since the job needs to installec-test-clifrom it)Assisted-by: GitHub Copilot:claude-opus-4.6