Skip to content

ci: build and test Node.js sample#126

Merged
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
ShuoRen-TT:agent/nodejs-build-test-ci
Jul 16, 2026
Merged

ci: build and test Node.js sample#126
damaz91 merged 1 commit into
Universal-Commerce-Protocol:mainfrom
ShuoRen-TT:agent/nodejs-build-test-ci

Conversation

@ShuoRen-TT

@ShuoRen-TT ShuoRen-TT commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a path-filtered GitHub Actions workflow for the Node.js sample
  • install dependencies with Node 20 and npm ci
  • require npm run build to pass
  • run npm test --if-present so current and future Node tests are covered

Why

The repository currently runs pre-commit checks but does not compile or test rest/nodejs. As a result, strict TypeScript errors in the Node.js sample remained undetected across dependency updates.

This workflow adds a focused build gate for changes under rest/nodejs/** without adding work to unrelated sample PRs.

Validation

Dependencies

This draft depends on #123 and #125. The new build job is expected to fail against the current main branch until both existing Node.js compile failures are merged. The workflow deliberately does not use continue-on-error; after those dependencies land, it becomes the regression gate that prevents the same drift from recurring.

@ShuoRen-TT
ShuoRen-TT marked this pull request as ready for review July 9, 2026 23:09
@damaz91 damaz91 added the status:needs-triage Signal that the PR is ready for human triage label Jul 9, 2026
vishkaty pushed a commit to vishkaty/samples that referenced this pull request Jul 9, 2026
Path-filter push/pull_request triggers to rest/python/** and the
workflow file itself, matching the path-filtered shape of the Node.js
workflow proposed in Universal-Commerce-Protocol#126, so unrelated changes do not spend CI minutes
booting the Python server.
@vishkaty

vishkaty commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Nice — this gives rest/nodejs the CI coverage that #120 proposes for rest/python (integration tests + server boot + happy-path client). I've borrowed this PR's path-filter shape for #120 so the two workflows stay consistent; happy to align on any other conventions that land here.

vishkaty pushed a commit to vishkaty/samples that referenced this pull request Jul 13, 2026
Path-filter push/pull_request triggers to rest/python/** and the
workflow file itself, matching the path-filtered shape of the Node.js
workflow proposed in Universal-Commerce-Protocol#126, so unrelated changes do not spend CI minutes
booting the Python server.
vishkaty pushed a commit to vishkaty/samples that referenced this pull request Jul 15, 2026
Path-filter push/pull_request triggers to rest/python/** and the
workflow file itself, matching the path-filtered shape of the Node.js
workflow proposed in Universal-Commerce-Protocol#126, so unrelated changes do not spend CI minutes
booting the Python server.
@ShuoRen-TT
ShuoRen-TT force-pushed the agent/nodejs-build-test-ci branch from 29a09bd to 0bcc1f2 Compare July 16, 2026 07:02
@ShuoRen-TT

Copy link
Copy Markdown
Contributor Author

Thanks for taking another look at this.

#123 and #125 have now merged. I rebased this PR onto the latest main (30213a3) and reran the exact workflow commands locally with Node.js v20.20.2:

The PR remains scoped to .github/workflows/nodejs.yml only. The CLA and security checks are passing. The Node.js Build and Test, Lint, and Conventional Commits runs currently show action_required because the fork workflow runs require maintainer approval.

When convenient, could a maintainer please approve those workflow runs and review the PR for the required governance approval? Thank you!

@damaz91 damaz91 added status:under-review and removed status:needs-triage Signal that the PR is ready for human triage labels Jul 16, 2026
@damaz91
damaz91 merged commit 65a18a2 into Universal-Commerce-Protocol:main Jul 16, 2026
14 checks passed
damaz91 added a commit that referenced this pull request Jul 16, 2026
* ci: add Python server integration-test and happy-path workflow

* ci: scope the Python server workflow to rest/python changes

Path-filter push/pull_request triggers to rest/python/** and the
workflow file itself, matching the path-filtered shape of the Node.js
workflow proposed in #126, so unrelated changes do not spend CI minutes
booting the Python server.

* ci: install ucp-sdk from pip in the Python server workflow

Follows samples#131: the server's pyproject now resolves ucp-sdk from pip, so
the workflow no longer checks out the python-sdk repo (and no longer needs the
nested checkout path that made the editable ../python-sdk reference work).
CI exercises the same released SDK a user installs; catching SDK-main drift
is python-sdk CI's job, not this workflow's.

* refactor(client): return non-zero exit code on failure in happy-path client

This allows the CI workflow to rely on the exit status of the client
instead of grepping the logs for success.

Simplified the workflow step accordingly.

---------

Co-authored-by: Vishal Katyal <vishal@katyal.ai>
Co-authored-by: vishkaty <vishkaty@users.noreply.github.com>
Co-authored-by: damaz91 <federico.damato91@gmail.com>
Co-authored-by: Federico D'Amato <damaz@google.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.

4 participants