Skip to content

Docs snippets tests - #538

Merged
techwritermat merged 13 commits into
masterfrom
docs-snippets-live-tests
Nov 6, 2026
Merged

techwritermat merged 13 commits into
masterfrom
docs-snippets-live-tests

Conversation

@techwritermat

Copy link
Copy Markdown
Contributor

No description provided.

@pubnub-snyk-universal-prd

pubnub-snyk-universal-prd Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

✅ Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
✅ Open Source Security 0 0 0 0 0 issues
✅ Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

techwritermat and others added 4 commits November 4, 2026 12:40
- Added a new test to verify occupant visibility through channel groups in presence.ts, addressing potential timing issues in CI.
- Refactored exitWhenIdle function in cli.ts to provide clearer error messages when the event loop remains busy, and introduced a new describeHandles function to detail active handles and resources.
…tions

- Updated the exitWhenIdle function in cli.ts to provide clearer warnings when only timers are keeping the event loop busy.
- Added unref() calls in wait.ts to ensure abandoned timers do not prevent process exit after tests complete.
The presence fixture now waits until each channel group (cg1,
my_channel_group) and the ch1 plus cg1 combination list the occupant.
A group read resolves the group's member channels first and can lag
the per-channel reads, which made hereNowBasicUsage fail in CI.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Five steps read back what the docs sample just wrote, from another
client, in one shot. App Context reads lag behind writes, and on the
fast CI runner setMembershipBasicUsage read an empty list. The same
assertions now run inside eventually().

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
techwritermat and others added 3 commits November 5, 2026 09:41
- Introduced a new script `snippet-summary.cjs` to generate a Markdown summary of snippet results per ID for GitHub Actions.
- Updated `package.json` to include a new command for running the summary script.
- Modified GitHub Actions workflows to utilize the new summary script and adjusted checkout actions for consistency.
- Enhanced error handling in `mocha-entry.ts` to ensure all keysets are processed, even if some fail.

This improves visibility into snippet test results and ensures comprehensive reporting in CI runs.
- Revised multiple snippets and README entries to specify that React and React Native sources cannot be run in this repository due to the absence of necessary dependencies.
- Adjusted comments and documentation to reflect that the snippets are retained for documentation purposes only and are type-checked but not executed.
- Enhanced clarity in various files regarding the environment setup and execution context for the snippets.

This improves the accuracy of documentation and helps users understand the limitations of running certain snippets.

@parfeon parfeon 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.

Mostly looks good.

Comment thread .github/workflows/docs-snippets.yml Outdated
live: ${{ steps.diff.outputs.live }}
steps:
- name: Checkout repository
uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4

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.

this is brutal. We actually can at least trust official GitHub account to deliver stable products.
Current checkout is v7

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.

changed

Comment thread .github/workflows/docs-snippets.yml Outdated
# of its own role only. The janitor step after them needs every role, so it keeps all of them.
- name: Shard open
id: open
if: success() || failure()

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.

so, we will still run this step event previous setup steps failed? Does it really make sense?

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.

I needed it for testing as one of the tests was flaky and it never ran my code snippets check. Now it runs only when the build step succeeded and the run wasn't cancelled

setPresenceState();
}, []);

return <div>User with presence state</div>;

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.

Should it show progress as it has been shown with react-subscribe.jsx?

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.

changed

Comment on lines +43 to +44
const channel = pubnub.channel('hello_world');
const subscription = channel.subscription();

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.

Should we show it in the following way:

Suggested change
const channel = pubnub.channel('hello_world');
const subscription = channel.subscription();
const subscription = pubnub.channel('hello_world').subscription();

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.

changed

<head>
<meta charset="utf-8" />
<title>PubNub quickstart</title>
<script src="https://cdn.pubnub.com/sdk/javascript/pubnub.13.1.3.js"></script>

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.

This could become invalid really soon.
Version bumping available through the release process but it requires modification in this file.

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.

Good catch. The release process now updates the CDN version in quickstart-web.html along with .pubnub.yml, package.json and README.md, like you suggested.

I also fixed two stale 9.6.0 references in the shared worker sample, the comment and the worker URL. The same release config updates them.

Review fixes (PR 538):
- Workflow: checkout moves to v7. Shards run only after a successful build. Keep-report steps run after a cancel too. The janitor runs only after a build, so nothing is revoked for a run that never started.
- react-subscribe.jsx: the subscribed text shows only after the client reports PNConnectedCategory or PNSubscriptionChangedCategory, through pubnub.addListener (the subscription object has no onStatus).
- react-set-state.jsx: shows "Setting state..." until setState resolves.
- quickstart-web.ts and .html: one chained line for the subscription.

Policy: every skip fails the run. The budget, the allowed list, REQUIRE_FEATURES and STRICT are gone, and so are the browser and React Native stub steps that could never run here.

Not changed, open: the published declarations (lib/types) lack subscriptionWorkerUrl and declare toFile(): Promise<void>. The web source does not. The hidden @ts-expect-error lines stay until the published declarations are fixed.
- Updated the version in browser-shared-worker.ts from v9.6.0 to v13.1.3 to reflect the latest SDK version.
- Added new version patterns for quickstart-web.html and browser-shared-worker.ts in versions.json to enhance version matching capabilities.

This ensures that the documentation snippets are aligned with the current SDK version and improves version management in the documentation.
@techwritermat
techwritermat force-pushed the docs-snippets-live-tests branch from 32e31a4 to db0f637 Compare November 6, 2026 07:49
- Updated the presence test to include a polling mechanism that waits for the channel group member to appear in the results after the fixture joins.
- This change addresses timing issues in CI where the group member may not be immediately listed, ensuring more reliable test outcomes.

This improvement enhances the robustness of the presence tests by accommodating potential delays in member visibility.
- Implemented a retry logic for the joiner subscription to handle potential delays or losses in join events. The joiner will attempt to re-subscribe up to two times if the initial join event is not received.
- This enhancement ensures that lost joins are recorded and visible in the report, improving the reliability of the subscription process.

This change addresses issues with join event delivery and enhances the robustness of the subscribe legacy API.
@parfeon
parfeon self-requested a review November 6, 2026 09:18

@parfeon parfeon 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.

LGTM!

The snippet files are test fixtures for the docs. Their code is not
product code, and Codacy reports on them as issues.
@techwritermat
techwritermat merged commit 2ec1761 into master Nov 6, 2026
23 of 37 checks passed
@techwritermat
techwritermat deleted the docs-snippets-live-tests branch November 6, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants