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

test: compare local benchmark to latest and exit 1 #7529

Closed
wants to merge 4 commits into from

Conversation

ricokahler
Copy link
Contributor

Description

This PR refactors the benchmark suite to compare the performance between the local version of the sanity package and the latest published version from npm. The key changes introduced are:

  • Downloading and Using Latest sanity Package: The benchmark suite now automatically downloads the latest version of the sanity package as a tarball, extracts it, and aliases it in the Vite build process. This allows us to run benchmarks against both the local and the latest versions.

  • Running Benchmarks for Both Versions: For each test in the suite, benchmarks are executed twice—once using the local sanity package and once using the latest version. This provides a direct comparison of performance metrics between the two versions.

  • Comparing and Displaying Results: The results from both runs are collected and percentage differences are calculated. These differences are displayed in a table format, showing improvements or regressions in performance.

  • Exiting on Significant Regression: If the benchmark detects a performance regression exceeding 50% (i.e., the local version performs worse than the latest version by more than 50%), the script exits with an exit code of 1. This helps in automated environments to catch significant performance issues early.

Why are these changes introduced?

These changes are introduced to:

  • Enable developers to easily compare the performance of their current branch against the main branch or the latest published version.
  • Detect significant performance regressions early in the development process.
  • Integrate performance benchmarking more tightly into the development and CI/CD workflow.

What issue(s) does this solve?

  • Streamlines performance comparisons between different versions of the sanity package.
  • Automates detection of significant performance regressions.
  • Enhances the reliability of the benchmark suite by providing consistent and comparable results.

What to review

Steps for Reviewers:

  1. Code Changes:

    • Review the changes in index.ts and runTest.ts.
    • Ensure that the logic for downloading and aliasing the latest sanity package is sound and doesn't introduce side effects.
    • Check the modifications in the Vite build configuration to confirm that the correct version of sanity is used in each benchmark run.
  2. Benchmark Execution:

    • Verify that the benchmarks are running twice for each test—once with the local version and once with the latest version.
    • Ensure that the percentage differences are calculated accurately and displayed correctly in the output table.
  3. Exit Conditions:

    • Test scenarios where performance regressions exceed the 50% threshold to confirm that the script exits with code 1.
    • Check that normal execution (without significant regressions) completes successfully without unintended exits.

Affected Areas:

  • Benchmark Suite Scripts: Specifically index.ts and runTest.ts.
  • Build and Execution Flow: The way the benchmarks are built and executed has been modified to accommodate dual runs and comparisons.
  • CI/CD Integration: The exit code behavior may affect CI pipelines that rely on the benchmark suite.

Testing

Testing Performed:

  • Manual Testing:
    • Ran the benchmark suite locally to ensure that it correctly downloads the latest sanity package and aliases it in the build.
    • Verified that benchmarks are executed for both versions and that results are collected without errors.
    • Checked the calculation of percentage differences by comparing known performance metrics.
    • Tested the exit behavior by artificially introducing performance regressions exceeding 50% and confirming that the script exits with code 1.

Automated Testing:

  • No automated tests were added for this change due to the nature of the benchmark suite and the difficulty in simulating performance regressions in a controlled automated environment.
  • The changes rely heavily on external dependencies and runtime performance, which are challenging to test automatically without introducing flakiness.

Reasoning:

  • Given the complexity and runtime dependencies, manual testing was deemed sufficient to validate the functionality.
  • The focus was on ensuring that the benchmark suite behaves correctly in practical use cases.

Notes for release

What Changed:

  • The benchmark suite now compares performance between the local development version and the latest published version of the sanity package.
  • It automatically downloads the latest sanity package and includes it in the benchmark runs.
  • The suite calculates and displays percentage differences in performance metrics between the two versions.
  • If a performance regression exceeding 50% is detected, the suite exits with an exit code of 1.

How to Use It:

  • Run the benchmark suite as usual using the npm start or npm test commands.
  • The suite will handle downloading the latest sanity package and running the benchmarks for both versions automatically.
  • Upon completion, the results will be displayed in the console, showing performance metrics and percentage differences.

Example Output:

CleanShot 2024-09-19 at 14 30 32@2x

Are there limitations?

  • Environment Dependency: The performance metrics can vary depending on the machine and environment where the benchmarks are run.
  • Exit Behavior: The script will exit with code 1 if a performance regression exceeding 50% is detected. This may affect CI/CD pipelines and should be handled appropriately.
  • Benchmark Scope: The comparisons are limited to the tests included in the suite. Additional tests may be needed to cover all use cases.

Not Required for Release Notes:

  • This change enhances internal tooling and does not affect end-users directly.
  • No changes to public APIs or user-facing features.

Copy link

vercel bot commented Sep 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2024 9:24pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 20, 2024 9:24pm
4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Sep 20, 2024 9:24pm
page-building-studio ⬜️ Skipped (Inspect) Sep 20, 2024 9:24pm
test-compiled-studio ⬜️ Skipped (Inspect) Sep 20, 2024 9:24pm
test-next-studio ⬜️ Skipped (Inspect) Sep 20, 2024 9:24pm

@ricokahler ricokahler marked this pull request as ready for review September 19, 2024 19:31
@ricokahler ricokahler requested a review from a team as a code owner September 19, 2024 19:31
Copy link
Contributor

No changes to documentation

Copy link
Contributor

github-actions bot commented Sep 19, 2024

Component Testing Report Updated Sep 20, 2024 9:37 PM (UTC)

✅ All Tests Passed -- expand for details
File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 45s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 9s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 31s 6 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 37s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 18s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 10s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 3m 0s 0 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 45s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 42s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 16s 18 0 0
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 9s 3 9 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 26s 9 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 18s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 36s 12 0 0
formBuilder/tree-editing/TreeEditing.spec.tsx ✅ Passed (Inspect) 0s 0 3 0
formBuilder/tree-editing/TreeEditingNestedObjects.spec.tsx ✅ Passed (Inspect) 0s 0 3 0

@vercel vercel bot temporarily deployed to Preview – test-compiled-studio September 20, 2024 21:24 Inactive
@vercel vercel bot temporarily deployed to Preview – test-next-studio September 20, 2024 21:24 Inactive
@vercel vercel bot temporarily deployed to Preview – page-building-studio September 20, 2024 21:24 Inactive
@ricokahler
Copy link
Contributor Author

superseded by #7556

@ricokahler ricokahler closed this Sep 29, 2024
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.

1 participant