-
Notifications
You must be signed in to change notification settings - Fork 553
Test debugAsserts are removed from bundle #24044
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
Test debugAsserts are removed from bundle #24044
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR automates the testing of debug assert removal from production bundles by introducing a mocha test and updating related configuration and source files.
- Adds a mocha test to inspect the contents of the production bundle for expected and unexpected strings.
- Updates the debug assert file to include an extra assertion and error message for verification.
- Updates Mocha and ESLint configuration files to support the test setup.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
File | Description |
---|---|
examples/utils/bundle-size-tests/src/test/checkDebugAsserts.spec.ts | Introduces a mocha test to check the bundle contents. |
examples/utils/bundle-size-tests/.mocharc.cjs | Configures mocha test specs for the bundle tests. |
examples/utils/bundle-size-tests/.eslintrc.cjs | Adds ESLint overrides for test files. |
examples/utils/bundle-size-tests/src/debugAssert.ts | Updates import and content for debug assert verification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good beyond the one comment--feel free to ping for reapproval if I'm not prompt :)
Co-authored-by: Abram Sanderson <Abram.sanderson@gmail.com>
## Description Fixes the pipeline in the public ADO project that calculates bundle size baselines, which [broke](https://dev.azure.com/fluidframework/public/_build?definitionId=48&_a=summary) when we merged #24044. The root cause is that a test introduced in that PR depends on webpack having run on the bundle-size-tests package to produce a bundle, which the test then checks. The main `build - client pipeline` happens to run webpack after the build script, but the pipeline that calculates bundle size baselines does not, and thus it started failing. The way this PR fixes the pipeline is by making it so it *does not run the repo tests*, since it doesn't have any reason to. That's already taken care of by `build - client packages`. That also saves us a lot of runtime for it:  ### Side discussion The PR that introduced the test tried to ensure webpack ran [by declaring fluid-build dependencies](https://github.com/microsoft/FluidFramework/blob/529fae8207ab32e113b8d5b45f0ade4dd01f0313/examples/utils/bundle-size-tests/package.json#L85-L90); but we don't seem to use fluid-build to run tests anywhere, so this didn't have any effect. Do we need to update our test npm scripts so they use fluid-build, like our build ones do? It's not entirely clear to me that we can easily do that.
Description
Automate previously manual checks for debugAssert removal as a mocha test.
Split out from #24013
Reviewer Guidance
The review process is outlined on this wiki page.