Skip to content

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

Merged
merged 4 commits into from
Mar 17, 2025

Conversation

CraigMacomber
Copy link
Contributor

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.

@Copilot Copilot AI review requested due to automatic review settings March 12, 2025 21:17
Copy link
Contributor

@Copilot Copilot AI left a 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.

@github-actions github-actions bot added area: examples Changes that focus on our examples dependencies Pull requests that update a dependency file base: main PRs targeted against main branch labels Mar 12, 2025
Copy link
Contributor

@Abe27342 Abe27342 left a 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 :)

@CraigMacomber CraigMacomber merged commit 9466117 into microsoft:main Mar 17, 2025
33 checks passed
@CraigMacomber CraigMacomber deleted the TestDebugAssertRemoval branch March 17, 2025 20:13
alexvy86 added a commit that referenced this pull request Mar 24, 2025
## 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:


![image](https://github.com/user-attachments/assets/11e5c26d-7b30-4475-94d1-1af7874f9017)

### 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: examples Changes that focus on our examples base: main PRs targeted against main branch dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants