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

fix: fix jest timeout in cli cache tests #876

Merged
merged 1 commit into from
Jan 29, 2025
Merged

fix: fix jest timeout in cli cache tests #876

merged 1 commit into from
Jan 29, 2025

Conversation

mellyeliu
Copy link
Member

@mellyeliu mellyeliu commented Jan 29, 2025

What changed / motivation ?

In the refactor here #804 we should be wrapping done() in a finally block to prevent the test from hanging if an error is thrown.

Currently, if an assertion fails (e.g., an expect() check fails), the test times out instead of displaying the failure, since done() is never called.

With try/catch/finally:

  • Assertions properly fail the test (catch ensures done(err) is called)
  • Test always completes, even if an error occurs

This prevents Jest from hanging indefinitely and ensures failures are reported correctly

Testing

Tests pass on main.

In #875, tests no longer timeout, but accurately display the error:

expect(received).toEqual(expected) // deep equality

    - Expected  - 19
    + Received  + 18

    @@ -14,30 +14,29 @@
      import npmStyles from './npmStyles';
      import "./stylex_bundle.css";
      const fadeAnimation = "xgnty7z-B";
      const styles = {
        foo: {
    -     kKVMdj: "xeuoslp",
    +     animationName: "xeuoslp",
    -     k1xSpc: "x78zum5",
    +     display: "x78zum5",
    -     keTefX: "x1hm9lzh",
    +     marginInlineStart: "x1hm9lzh",
    -     koQZXg: null,
    +     marginLeft: null,
    -     km5ZXQ: null,
    +     marginRight: null,
    -     keoZOQ: "xlrshdv",

...

Pre-flight checklist

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 29, 2025
Copy link

github-actions bot commented Jan 29, 2025

workflow: benchmarks/size

Comparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.

@stylexjs/[email protected] size:compare
./size-compare.js /tmp/tmp.ZTL2RvaHvG /tmp/tmp.cFuC3E9qFA

Results Base Patch Ratio
stylex/lib/stylex.js
· compressed 1,013 1,013 1.00
· minified 3,305 3,305 1.00
stylex/lib/StyleXSheet.js
· compressed 1,266 1,266 1.00
· minified 3,776 3,776 1.00
rollup-example/.build/bundle.js
· compressed 567,110 567,110 1.00
· minified 10,232,512 10,232,512 1.00
rollup-example/.build/stylex.css
· compressed 100,628 100,628 1.00
· minified 755,718 755,718 1.00

@mellyeliu mellyeliu merged commit 47a26ec into main Jan 29, 2025
8 checks passed
@mellyeliu mellyeliu deleted the cli-fix-async branch January 29, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants