test(client-PropertyDDS): mocha standardization#26804
Open
jason-ha wants to merge 2 commits intomicrosoft:mainfrom
Open
test(client-PropertyDDS): mocha standardization#26804jason-ha wants to merge 2 commits intomicrosoft:mainfrom
jason-ha wants to merge 2 commits intomicrosoft:mainfrom
Conversation
Use common mocha configuration pattern. - delete unused `property-properties/src/test/mocha.opts` - replace `.mocharc.json` (incorrect settings) with `.mocharc.cjs` - property-properties test setup .js module relocated away from `src/test` to `test` as build is not needed - package.json script entries updated to reflect CJS/ESM specifics Test cases now execute with common `[CJS]` prefix. Warnings about unmatched file patterns are gone: ```text Warning: Cannot find any files matching pattern "test/setup.js" Warning: Cannot find any files matching pattern "test/**/*.spec.js" ``` Note that property-properties test source files use `require` (CommonJS) and require conversion to test ESM (which is also not properly converted to support ESM). (The production ESM build is a phony and it not used.) Additionally suppress blame history for PropertyDDS whitespace formatting commit.
Contributor
There was a problem hiding this comment.
Pull request overview
Standardizes Mocha configuration in the PropertyDDS client packages to align with the repo-wide @fluid-internal/mocha-test-setup conventions, removing legacy mocha config files and updating test/build scripts accordingly.
Changes:
- Replaced per-package
.mocharc.json/mocha.optsusage with.mocharc.cjsbacked by@fluid-internal/mocha-test-setup/mocharc-common. - Updated
package.jsonscripts to split mocha execution by module system (CJS/ESM) and align with commonFLUID_TEST_MODULE_SYSTEMbehavior. - Added a
.git-blame-ignore-revsentry to suppress formatting-only blame noise for PropertyDDS.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| experimental/PropertyDDS/packages/property-properties/test/setup.cjs | Minor setup adjustment/comment around chai should() initialization. |
| experimental/PropertyDDS/packages/property-properties/src/test/mocha.opts | Deleted unused legacy mocha opts file. |
| experimental/PropertyDDS/packages/property-properties/package.json | Updated build/test scripts for common mocha config + CJS/ESM split. |
| experimental/PropertyDDS/packages/property-properties/.mocharc.json | Removed incorrect JSON mocha config. |
| experimental/PropertyDDS/packages/property-properties/.mocharc.cjs | Added shared mocha config wiring + requires test setup. |
| experimental/PropertyDDS/packages/property-common/package.json | Updated build/test scripts for common mocha config + CJS/ESM split. |
| experimental/PropertyDDS/packages/property-common/.mocharc.json | Removed incorrect JSON mocha config. |
| experimental/PropertyDDS/packages/property-common/.mocharc.cjs | Added shared mocha config wiring. |
| .git-blame-ignore-revs | Added ignore entry for a PropertyDDS formatting commit. |
Comments suppressed due to low confidence (1)
experimental/PropertyDDS/packages/property-properties/test/setup.cjs:20
- The inline comment says the
chai.should()assignment shouldn't be needed, but the code still assigns to a globalshould. Since the tests appear to rely on the side-effect of enabling.shouldrather than theshouldvariable itself, consider changing this to just callchai.should()(no assignment) or removing the comment if the assignment is intentionally kept.
experimental/PropertyDDS/packages/property-properties/package.json
Outdated
Show resolved
Hide resolved
(that were added for future standardization)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use common mocha configuration pattern.
property-properties/src/test/mocha.opts.mocharc.json(incorrect settings) with.mocharc.cjssrc/testtotestas build is not neededTest cases now execute with common
[CJS]prefix. Warnings about unmatched file patterns are gone:Note that property-properties test source files use
require(CommonJS) and require conversion to test ESM (which is also not properly converted to support ESM). (The production ESM build is a phony and it not used.)Additionally suppress blame history for PropertyDDS whitespace formatting commit.
This further applies the changes started by #25503 and #25292 for client workspace.