Skip to content

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

Merged
aabmass merged 2 commits into
googleapis:mainfrom
aabmass:push-stsosypynvny
Jul 8, 2026
Merged

fix(cloud-profiler): re-enable tests#8828
aabmass merged 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 3 times, most recently 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
@aabmass
aabmass force-pushed the push-stsosypynvny branch from d09e658 to 84a96ab Compare July 8, 2026 15:20
@aabmass
aabmass merged commit be6bfb4 into googleapis:main Jul 8, 2026
35 checks passed
@aabmass
aabmass deleted the push-stsosypynvny branch July 8, 2026 16:48
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.

2 participants