Skip to content

fix(cloud-profiler): re-enable tests#8828

Open
aabmass wants to merge 2 commits into
googleapis:mainfrom
aabmass:push-stsosypynvny
Open

fix(cloud-profiler): re-enable tests#8828
aabmass wants to merge 2 commits into
googleapis:mainfrom
aabmass:push-stsosypynvny

Conversation

@aabmass

@aabmass aabmass commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Restores the test (unit test) script that was previously disabled in googleapis/cloud-profiler-nodejs#1002.

Note: GCE system tests (system-test) remain disabled for now as they require a GCE project environment to run.

Note: Native module compilation (via node-gyp) fails on GitHub Actions Windows runners due to runner-specific environment issues. Consequently, handwritten/cloud-profiler/ has been added to the Windows test exemption list in ci/run_conditional_tests.sh.

@aabmass aabmass requested a review from a team as a code owner July 7, 2026 18:41
@product-auto-label product-auto-label Bot added the api: cloudprofiler Issues related to the Cloud Profiler API. label Jul 7, 2026
@aabmass aabmass force-pushed the push-stsosypynvny branch 2 times, most recently from f5ade47 to d5ced77 Compare July 7, 2026 18:45

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates the package.json file in handwritten/cloud-profiler to enable unit and system tests using mocha and c8 instead of skipping them. Feedback is provided to wrap the glob patterns in single quotes to ensure cross-platform compatibility, particularly on Windows where shell glob expansion behaves differently.

Comment thread handwritten/cloud-profiler/package.json Outdated
@aabmass aabmass force-pushed the push-stsosypynvny branch 4 times, most recently from 9341bad to d2a0217 Compare July 7, 2026 20:44
@aabmass aabmass requested a review from a team as a code owner July 7, 2026 20:44
@aabmass aabmass force-pushed the push-stsosypynvny branch 2 times, most recently from 16321fe to c530845 Compare July 7, 2026 21:42
@aabmass aabmass marked this pull request as draft July 7, 2026 21:44
@aabmass aabmass force-pushed the push-stsosypynvny branch 2 times, most recently from 98934c5 to 955d2d6 Compare July 7, 2026 23:23
Restores the `test` and `system-test` scripts that were previously disabled in googleapis/cloud-profiler-nodejs#1002.
@aabmass aabmass force-pushed the push-stsosypynvny branch from 955d2d6 to ec2b64a Compare July 7, 2026 23:24

# Our CI uses Git Bash on Windows to execute this script, which returns "msys" for OSTYPE.
if [[ "$OSTYPE" == "msys" ]]; then
# Our CI uses Git Bash on Windows to execute this script, which returns "msys" or "cygwin" for OSTYPE.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Turned out $OSTYPE is set to cygwin on the Windows GHA runners, which bypassed the original msys check. Adding cygwin and the $OS fallback makes sure we skip Windows-exempt tests correctly on both GHA and local setups.

  • Failed run where it triggered anyway link
  • Succeeded run with echo showing the value of OSTYPE link

@aabmass aabmass marked this pull request as ready for review July 8, 2026 02:35
@aabmass aabmass requested a review from psx95 July 8, 2026 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: cloudprofiler Issues related to the Cloud Profiler API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant