Skip to content

Enforce example registration in test-examples#941

Open
dktsudgg wants to merge 3 commits into
fedify-dev:mainfrom
dktsudgg:886-test-unregistered-example-detection
Open

Enforce example registration in test-examples#941
dktsudgg wants to merge 3 commits into
fedify-dev:mainfrom
dktsudgg:886-test-unregistered-example-detection

Conversation

@dktsudgg

@dktsudgg dktsudgg commented Jul 15, 2026

Copy link
Copy Markdown

Closes #886

Summary

  • Add registry-consistency tests that fail when an example in the examples/ directory is unregistered or a registered example has no matching directory.
  • Make examples/test-examples/ a root Deno workspace member so that CI picks up the added tests.
  • Register the previously missing solidstart example, and restore the cmd field of MultiHandleExample that a formatting sweep (74d18f0) accidentally renamed.

Test plan

  • Tested with deno test --allow-all examples/test-examples and mise test
  • mkdir examples/zz-fake && deno test --allow-all examples/test-examples fails, naming the unregistered directory

Test Screenshots

  1. When an example in the examples/ directory is unregistered
스크린샷 2026-07-16 오전 2 00 12
  1. When a registered example has no matching directory
스크린샷 2026-07-16 오전 2 03 33

AI assistance disclosure: Claude Code (claude-fable-5) helped implement and verify the change.

dktsudgg added 2 commits July 15, 2026 23:27
The commit that refined em dash spacing (74d18f0) accidentally
renamed the `cmd` field to `cmdPrefix`, so this commit restores it.

Assisted-by: Claude Code:claude-fable-5
Background:

`test-examples` promises that every directory under examples/ is
registered in one of its registries, but it only printed a warning
when one was not, so a new example could silently miss automated
checks.  In fact, examples/solidstart had never been registered.

Changes:

- Add registry-consistency tests that fail when an example in the
  examples/ directory is unregistered or a registered example has
  no matching directory.
- Make examples/test-examples a root Deno workspace member so that
  CI picks up the added tests.
- Register the solidstart example, which the new test caught as
  unregistered.

Closes fedify-dev#886

Assisted-by: Claude Code:claude-fable-5
@netlify

netlify Bot commented Jul 15, 2026

Copy link
Copy Markdown

Deploy Preview for fedify-json-schema canceled.

Name Link
🔨 Latest commit 2d186c2
🔍 Latest deploy log https://app.netlify.com/projects/fedify-json-schema/deploys/6a59969b95587200099d2c34

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The example test runner registers a SolidStart example, exposes registry-scanning helpers, supports safe imports, and adds tests for missing or unregistered example directories. Deno workspace and task configuration supports running the focused tests and runner.

Changes

Example registry validation

Layer / File(s) Summary
Registry contracts and entries
examples/test-examples/mod.ts
The multi-handle configuration requires cmd, and the server registry adds the solidstart example with its commands and readiness settings.
Runner scanning and execution wiring
examples/test-examples/mod.ts, deno.json, examples/test-examples/deno.json
Exported helpers scan registered and filesystem examples, main() uses the scanner, execution is guarded by import.meta.main, and Deno workspace tasks are configured.
Registry consistency tests
examples/test-examples/mod.test.ts
Tests verify that no example directories are unregistered and no registered examples are missing from the filesystem.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: examples

Suggested reviewers: dahlia

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR adds the requested check for unregistered examples and reports missing matches clearly, satisfying #886.
Out of Scope Changes check ✅ Passed The additional workspace, example registry, and helper refactors all support the stated goal and do not appear unrelated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly summarizes the main change: enforcing example registration in test-examples.
Description check ✅ Passed The description is directly related to the changeset and accurately describes the registry tests, workspace update, and example fixes.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@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 registers a new solidstart example and introduces a test suite (test-examples) to ensure consistency between the physical directories in examples/ and the registered examples in mod.ts. The review feedback recommends ignoring hidden directories (such as .DS_Store or .git) and decoupling logical example names from physical directory names by introducing and checking against a getRegisteredExampleDirs() helper instead of getRegisteredExampleNames(). All review comments are constructive and provide actionable code suggestions to improve test robustness.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread examples/test-examples/mod.ts
import assert from "node:assert/strict";
import test from "node:test";

import { getRegisteredExampleNames, scanUnregisteredExamples } from "./mod.ts";

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.

high

Update the import to use getRegisteredExampleDirs instead of getRegisteredExampleNames to support checking directory names rather than logical example names.

Suggested change
import { getRegisteredExampleNames, scanUnregisteredExamples } from "./mod.ts";
import { getRegisteredExampleDirs, scanUnregisteredExamples } from "./mod.ts";

Comment thread examples/test-examples/mod.test.ts

@2chanhaeng 2chanhaeng left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! I left a little suggestion. And next time you open another PR, would you please mention the related issue at the very beginning of the PR body? It really helps with understanding the context faster.

Comment thread examples/test-examples/mod.ts Outdated
Comment on lines +367 to +372
return new Set([
...SERVER_EXAMPLES.map((e) => e.name),
...SCRIPT_EXAMPLES.map((e) => e.name),
...MULTI_HANDLE_EXAMPLES.map((e) => e.name),
...SKIPPED_EXAMPLES.map((e) => e.name),
]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it will be better like this:

  return new Set([
    ...SERVER_EXAMPLES,
    ...SCRIPT_EXAMPLES,
    ...MULTI_HANDLE_EXAMPLES,
    ...SKIPPED_EXAMPLES,
  ].map((e) => e.name))

@dktsudgg dktsudgg Jul 17, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@2chanhaeng Thank you for your review! I added a new commit 2d186c2

I also moved the comment about the related issue to the top of PR

Combine the registered example arrays before mapping their names to avoid repeated map operations.
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.

Test unregistered example detection

2 participants