Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove CommonJS Testing #19868

Merged
merged 4 commits into from
Feb 29, 2024
Merged

Remove CommonJS Testing #19868

merged 4 commits into from
Feb 29, 2024

Conversation

sonalideshpandemsft
Copy link
Contributor

@sonalideshpandemsft sonalideshpandemsft commented Feb 28, 2024

This PR eliminates CJS testing, resulting in nearly double the execution time for Mocha tests. This increase in time may stem from testing both cjs and esm module.

@github-actions github-actions bot added area: dds Issues related to distributed data structures area: driver Driver related issues area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: odsp-driver area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch labels Feb 28, 2024
@sonalideshpandemsft sonalideshpandemsft changed the title Test esm ESM test for packages Feb 28, 2024
@sonalideshpandemsft sonalideshpandemsft changed the title ESM test for packages Remove CommonJS Testing Feb 28, 2024
@sonalideshpandemsft sonalideshpandemsft marked this pull request as ready for review February 28, 2024 19:34
Copy link
Contributor

@jason-ha jason-ha left a comment

Choose a reason for hiding this comment

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

There are also esm then cjs scripts.
But we should see how this moves the overhead needle. If this group of changes is significant, then we don't need to do more. If it is not noticeable, then we know we should look at the other group.

experimental/dds/ot/ot/package.json Outdated Show resolved Hide resolved
@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Feb 28, 2024

@fluid-example/bundle-size-tests: +154 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 514.16 KB 514.2 KB +44 Bytes
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 247.99 KB 248.01 KB +22 Bytes
loader.js 171.3 KB 171.32 KB +22 Bytes
map.js 46.69 KB 46.7 KB +11 Bytes
matrix.js 148.74 KB 148.74 KB No change
odspDriver.js 97.31 KB 97.33 KB +22 Bytes
odspPrefetchSnapshot.js 42.28 KB 42.29 KB +11 Bytes
sharedString.js 167.32 KB 167.33 KB +11 Bytes
sharedTree.js 334.63 KB 334.63 KB No change
Total Size 1.87 MB 1.87 MB +154 Bytes

Baseline commit: bdef6d1

Generated by 🚫 dangerJS against 9c24b2d

@sonalideshpandemsft
Copy link
Contributor Author

There are also esm then cjs scripts. But we should see how this moves the overhead needle. If this group of changes is significant, then we don't need to do more. If it is not noticeable, then we know we should look at the other group.

I didn't find any script which esm followed by cjs. For tree dds, esm testing is skipped: https://github.com/microsoft/FluidFramework/blob/main/packages/dds/tree/package.json#L71

Copy link
Contributor

@jason-ha jason-ha left a comment

Choose a reason for hiding this comment

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

I was thinking of the build:test ordering. Ship it.

@jason-ha jason-ha merged commit 42b17b4 into main Feb 29, 2024
31 checks passed
@jason-ha jason-ha deleted the remove-cjs-build branch February 29, 2024 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: dds Issues related to distributed data structures area: driver Driver related issues area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: odsp-driver area: tests Tests to add, test infrastructure improvements, etc base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants