Fix coverage link, streamline env vars in CI by ggetz · Pull Request #12971 · CesiumGS/cesium

ggetz · 2025-10-10T17:31:11Z

Description

TLDR; This PR should fix the link to coverage results in GitHub PRs, but more broadly, does some housekeeping of our deploy and status scripts.

Mismatched environment variables

Between incremental updates and migrating between various services and tools, our build and deploy scripts have become quite sprawling. Some things also fall through the cracks, like the broken test coverage link in the CI checks, but we're also dragging along patterns that are no longer serving us. This PR does not fix everything, but attempts to consolidate and cleanup some of these CI/CD related things.

I noticed when reviewing #12957 that there are several misconfigured environment variables in the dev workflow as opposed to the deploy workflow where most other build artifacts are uploaded. Mis-matches across the workflows were ultimately resulting in broken links.

When using the pull_request hook, the GitHub ref_name is different than the branch name. This ended up using different versions of "DEPLOYED_URL" leading to a broken link.
A similar issue arises when trying to use the commit SHA for operations like setting statuses. In pull_request triggers, the value of COMMIT_SHA will the merge commit.

Change summary

Does not change the production or dedicated sandcastle deployment workflows beyond removing redundant env properties.
Cleaned up some (quite old) deploy scripts:
- Moved the setDeployStatus script out of gulpfile.js as it is unrelated to the build processes—It is now a standalone node script with command line help docs.
- Replaced with setCommitStatus. For flexibility, the command itself is more granular so it can be run per-artifact and per-status update. This allows check updates to run separately in different tasks or workflows as needed.
- Removed the deploySetVersion script out of gulpfile.js in favor of the npm version cli command which we use for our release processes.
- Since they are generally only the concerns of the maintainers, these script have been removed from package.json and the corresponding build guide.
Moved the coverage job to it's own workflow, having it only run on pull requests triggers. This simplifies the job a lot and, to my knowledge, the check itself isn't super helpful until a PR is opened.
Created .env files in which can configure common environment variables as described in the updated docs of ContinuousIntegration/README.md
- This adds a dependency on dotenvx as it allows us to variable expansion and simple command substitution.
- Defines values accounting for variations in environments:
  - In CI, the trigger can be either push or pull_request. Since pull_request does not run in the context of the branch itself, we need to account for the available GitHub context to grab branch names and commit SHA's.
  - Locally, fall back to git commands to allow for development testing.
- Create a new env var, CESIUM_VERSION, that detects the current package version making it more convenient to access across various contexts.

Issue number and link

Fixes #11827

Testing plan

When CI runs, all expected jobs should start:

CI artifacts should then show up as pending:

CI artifacts should all transition to success and each link should work:
a. "coverage results" should navigate to the coverage results index
b. "cesium-.tgz" should download the packed npm package
c. "Cesium-.zip" should download the release zip
d. "index.html` should navigate to the deployed static build

Author checklist

I have submitted a Contributor License Agreement
I have added my name to CONTRIBUTORS.md
~~I have updated CHANGES.md with a short summary of my change~~
~~I have added or updated unit tests to ensure consistent code coverage~~
I have updated the inline documentation, and included code examples where relevant
I have performed a self-review of my code

github-actions · 2025-10-10T17:31:34Z

Thank you for the pull request, @ggetz!

✅ We can confirm we have a CLA on file for you.

ggetz · 2025-10-10T17:48:15Z

Wait to review; This needs an additional change.

ggetz · 2025-10-14T17:30:23Z

This is ready for review and the PR description is up-to-date.

jjspace

Overall very nice, thanks @ggetz! Definitely much better and more robust than the quick change I recently made. Just had one comment that's really a question

CI seems to be passing and working as expected. I assume there's not really any way to test this locally?

jjspace · 2025-10-14T18:47:39Z

+      - name: set status pending
+        if: ${{ env.AWS_ACCESS_KEY_ID != '' }}
+        run: |
+          npx @dotenvx/dotenvx run -f ./.github/workflows/.env.pull_request -- \


Why are we calling dotenvx from npx instead of using it directly in the script files? is it just so we can specify the extra .env.pull_request files?

Also as an aside for my curiosity, what is the reason you went with dotenvx instead of just dotenv? a quick search seems that the motivation would be to encrypt the env files with secrets but we aren't doing that. Is this future proofing?

I chose dotenvx specifically because it supports variable expansion and command substitution, which I don't believe "regular" dotenv supports.

Why are we calling dotenvx from npx instead of using it directly in the script files? is it just so we can specify the extra .env.pull_request files?

Yes. The default .env is being loaded from the script itself, but this command is what allows us to override with the CI-specific values in .env.pull_request. If there's an alternative syntax you know of, please tell!

I think my expectation is from using older versions of dotenv. I was used to the paradigm of always only including the package and configuring it in the file it's used in. It looks like this is still an option using dotenvx.config([path]). However I don't think this allows for the command line level variable substitution which it seems we need.

I think it's just kinda confusing not knowing where and when things are defined, especially when the commands are different in CI. I think I get it now but it's a bit weird.

For example, this command only uses -f ./.github/workflows/.env.pull_request because the setCommitStatus.js script imports @dotenvx/dotenvx/config to setup the "base" .env however the command lower in this file requires -f ./.github/workflows/.env.pull_request -f .env in order to include both.

Plus my other comment about not always being included could lead to devs getting confused why process.env vars are set up one way in some files but missing in others if they forget an import or to run the command wrapped with dotenvx run

Co-authored-by: jjspace <8007967+jjspace@users.noreply.github.com>

ggetz · 2025-10-14T20:12:05Z

CI seems to be passing and working as expected. I assume there's not really any way to test this locally?

No, not end-to-end. You could test some commands locally, but if CI is working as expected I think that's unnecessary.

ggetz · 2026-01-27T17:12:10Z

@jjspace Could you take a final pass on this PR and merge if happy? I think this addresses the rogue coverage artifacts you mentioned seeing.

jjspace

Overall I still think this is a good change. Just a few more comments/discussion around dotenvx

I also merged in main which I think still captures all the changes even though we broke apart the gulpfile some since this was first opened

jjspace · 2026-01-27T19:12:11Z

@@ -0,0 +1,132 @@
+import yargs from "yargs";
+import { hideBin } from "yargs/helpers";
+import "@dotenvx/dotenvx/config";


Should this be imported in the gulpfile and all our scripts?
Alternatively should all our package.json scripts be prefixed with dotenvx run?

jjspace · 2026-01-27T19:19:19Z

+      - name: set status pending
+        if: ${{ env.AWS_ACCESS_KEY_ID != '' }}
+        run: |
+          npx @dotenvx/dotenvx run -f ./.github/workflows/.env.pull_request -- \


I think my expectation is from using older versions of dotenv. I was used to the paradigm of always only including the package and configuring it in the file it's used in. It looks like this is still an option using dotenvx.config([path]). However I don't think this allows for the command line level variable substitution which it seems we need.

I think it's just kinda confusing not knowing where and when things are defined, especially when the commands are different in CI. I think I get it now but it's a bit weird.

For example, this command only uses -f ./.github/workflows/.env.pull_request because the setCommitStatus.js script imports @dotenvx/dotenvx/config to setup the "base" .env however the command lower in this file requires -f ./.github/workflows/.env.pull_request -f .env in order to include both.

Plus my other comment about not always being included could lead to devs getting confused why process.env vars are set up one way in some files but missing in others if they forget an import or to run the command wrapped with dotenvx run

jjspace · 2026-01-27T19:24:19Z

+          node-version: "22"
+      - name: npm install
+        run: npm install
+      - name: set status pending


What if these jobs fail? Do these pending statuses time out or fail automatically? I see we have a section at the end to set them to success but does the failure state have to be manual?

In the previous version the statuses weren't even created until it was successful.

Fix coverage link in CI

6780a16

Merge branch 'main' into coverage-link

cb1a73a

Centralize env vars in env; Refactor deployment checks

d771cf0

ggetz force-pushed the coverage-link branch from 377db1c to d771cf0 Compare October 13, 2025 19:49

ggetz and others added 3 commits October 13, 2025 16:31

Use correct workflow name

1ccaa77

Merge branch 'main' into coverage-link

ef2875d

Seperate .env per environment, docs

9a459e9

ggetz force-pushed the coverage-link branch from 36f1f7a to 9a459e9 Compare October 14, 2025 17:12

Merge branch 'main' into coverage-link

185c5a5

ggetz changed the title ~~Fix coverage link in CI~~ Fix coverage link in CI, streamline CI vars Oct 14, 2025

ggetz changed the title ~~Fix coverage link in CI, streamline CI vars~~ Fix coverage link, streamline env vars in CI Oct 14, 2025

jjspace self-requested a review October 14, 2025 18:33

jjspace reviewed Oct 14, 2025

View reviewed changes

Update .env

7a060c9

Co-authored-by: jjspace <8007967+jjspace@users.noreply.github.com>

Merge branch 'main' into coverage-link

ba274be

ggetz self-assigned this Oct 30, 2025

Merge branch 'main' into coverage-link

8f14cb7

Merge remote-tracking branch 'origin/main' into coverage-link

26bb064

jjspace reviewed Jan 27, 2026

View reviewed changes

jjspace mentioned this pull request Feb 20, 2026

Update Sandcastle iTwin share keys #13225

Closed

4 tasks

Uh oh!

Conversation

ggetz commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Mismatched environment variables

Change summary

Issue number and link

Testing plan

Author checklist

Uh oh!

github-actions Bot commented Oct 10, 2025

Uh oh!

ggetz commented Oct 10, 2025

Uh oh!

ggetz commented Oct 14, 2025

Uh oh!

jjspace left a comment

Choose a reason for hiding this comment

Uh oh!

jjspace Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

ggetz Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

ggetz Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jjspace Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ggetz commented Oct 14, 2025

Uh oh!

ggetz commented Jan 27, 2026

Uh oh!

jjspace left a comment

Choose a reason for hiding this comment

Uh oh!

jjspace Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

jjspace Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

jjspace Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ggetz commented Oct 10, 2025 •

edited

Loading

ggetz Oct 14, 2025 •

edited

Loading